Re: Corruption during WAL replay
Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
From: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
To: Teja Mupparti <tejeswarm@hotmail.com>
Cc: Andres Freund <andres@anarazel.de>,
Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "hexexpert@comcast.net" <hexexpert@comcast.net>
Date: 2020-04-13T06:24:55Z
Lists: pgsql-hackers
On Sat, 11 Apr 2020 at 09:00, Teja Mupparti <tejeswarm@hotmail.com> wrote:
>
> Thanks Andres and Kyotaro for the quick review. I have fixed the typos and also included the critical section (emulated it with try-catch block since palloc()s are causing issues in the truncate code). This time I used git format-patch.
>
I briefly looked at the latest patch but I'm not sure it's the right
thing here to use PG_TRY/PG_CATCH to report the PANIC error. For
example, with the following code you changed, we will always end up
with emitting a PANIC "failed to truncate the relation" regardless of
the actual cause of the error.
+ PG_CATCH();
+ {
+ ereport(PANIC, (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("failed to truncate the relation")));
+ }
+ PG_END_TRY();
And the comments of RelationTruncate() mentions:
/*
* We WAL-log the truncation before actually truncating, which means
* trouble if the truncation fails. If we then crash, the WAL replay
* likely isn't going to succeed in the truncation either, and cause a
* PANIC. It's tempting to put a critical section here, but that cure
* would be worse than the disease. It would turn a usually harmless
* failure to truncate, that might spell trouble at WAL replay, into a
* certain PANIC.
*/
As a second idea, I wonder if we can defer truncation until commit
time like smgrDoPendingDeletes mechanism. The sequence would be:
At RelationTruncate(),
1. WAL logging.
2. Remember buffers to be dropped.
At CommitTransaction(),
3. Revisit the remembered buffers to check if the buffer still has
table data that needs to be truncated.
4-a, If it has, we mark it as IO_IN_PROGRESS.
4-b, If it already has different table data, ignore it.
5, Truncate physical files.
6, Mark the buffer we marked at #4-a as invalid.
If an error occurs between #3 and #6 or in abort case, we revert all
IO_IN_PROGRESS flags on the buffers.
In the above idea, remembering all buffers having to-be-truncated
table at RelationTruncate(), we reduce the time for checking buffers
at the commit time. Since we acquire AccessExclusiveLock the number of
buffers having to-be-truncated table's data never increases. A
downside would be that since we can truncate multiple relations we
need to remember all buffers of each truncated relations, which is up
to (sizeof(int) * NBuffers) in total.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Harden TAP tests that intentionally corrupt page checksums.
- d09f765b9dcb 12.11 landed
- 6e9ffcf13a4a 13.7 landed
- 579cef5faf11 14.3 landed
- 3d263b09058a 11.16 landed
- 174877f1e344 15.0 landed
-
Fix possible recovery trouble if TRUNCATE overlaps a checkpoint.
- 412ad7a55639 15.0 landed
- 57f618310f83 10.21 landed
- 118f1a332b00 11.16 landed
- 3821d66a7b3e 12.11 landed
- 1ce14b6b2fe4 13.7 landed
- bbace5697df1 14.3 landed
-
Remember to reset yy_start state when firing up repl_scanner.l.
- ef9706bbc8ce 14.2 cited