public inbox for [email protected]
help / color / mirror / Atom feedFrom: Imseih (AWS), Sami <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: [BUG] Panic due to incorrect missingContrecPtr after promotion
Date: Fri, 25 Feb 2022 21:28:18 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
> After some investigation, I finally concluded that we should reset
> abortedRecPtr and missingContrecPtr at processing
> XLOG_OVERWRITE_CONTRECORD.
> --- a/src/backend/access/transam/xlogrecovery.c
> +++ b/src/backend/access/transam/xlogrecovery.c
> @@ -1953,6 +1953,11 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
LSN_FORMAT_ARGS(xlrec.overwritten_lsn),
timestamptz_to_str(xlrec.overwrite_time))));
> + /* We have safely skipped the aborted record */
> + abortedRecPtr = InvalidXLogRecPtr;
> + missingContrecPtr = InvalidXLogRecPtr;
> +
> /* Verifying the record should only happen once */
> record->overwrittenRecPtr = InvalidXLogRecPtr;
> }
> The last check in the test against "resetting aborted record" is no
> longer useful since it is already checked by
> 026_verwrite_contrecord.pl.
> regards.
+1 for this. Resetting abortedRecPtr and missingContrecPtr after the broken record is skipped in is cleaner. I also think it would make sense to move the " successfully skipped missing contrecord" to after we invalidate the aborted and missing record pointers, like below.
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1948,15 +1948,15 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
LSN_FORMAT_ARGS(xlrec.overwritten_lsn),
LSN_FORMAT_ARGS(record->overwrittenRecPtr));
+ /* We have safely skipped the aborted record */
+ abortedRecPtr = InvalidXLogRecPtr;
+ missingContrecPtr = InvalidXLogRecPtr;
+
ereport(LOG,
(errmsg("successfully skipped missing contrecord at %X/%X, overwritten at %s",
LSN_FORMAT_ARGS(xlrec.overwritten_lsn),
timestamptz_to_str(xlrec.overwrite_time))));
Also, instead of a new test, the current 026_overwrite_contrecord.pl test can include a promotion test at the end.
Attaching a new patch for review.
--
Sami Imseih
Amazon Web Services
Attachments:
[application/octet-stream] v4-0001-Fix-missing-continuation-record-after-standby-promot.patch (2.4K, ../[email protected]/2-v4-0001-Fix-missing-continuation-record-after-standby-promot.patch)
download | inline diff:
From a43c4dc01c27a07798c935cbc15260d1a668d3b9 Mon Sep 17 00:00:00 2001
From: "Sami Imseih (AWS)" <[email protected]>
Date: Fri, 25 Feb 2022 21:23:34 +0000
Subject: [PATCH 1/1] Fix "missing continuation record" after standby promotion
Invalidate abortedRecPtr and missingContrecPtr after a missing
continuation record is skipped on a standby. This fixes a PANIC
caused when a recently promoted standby attempts to write an
OVERWRITE_RECORD with an LSN of the previously read aborted record.
Author: Sami Imseih
Reviewed-by: Kyotaro Horiguchi <[email protected]>, Alvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/transam/xlogrecovery.c | 4 ++++
src/test/recovery/t/026_overwrite_contrecord.pl | 5 ++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index f9f212680b..bbb2d7fef5 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1948,6 +1948,10 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
LSN_FORMAT_ARGS(xlrec.overwritten_lsn),
LSN_FORMAT_ARGS(record->overwrittenRecPtr));
+ /* We have safely skipped the aborted record */
+ abortedRecPtr = InvalidXLogRecPtr;
+ missingContrecPtr = InvalidXLogRecPtr;
+
ereport(LOG,
(errmsg("successfully skipped missing contrecord at %X/%X, overwritten at %s",
LSN_FORMAT_ARGS(xlrec.overwritten_lsn),
diff --git a/src/test/recovery/t/026_overwrite_contrecord.pl b/src/test/recovery/t/026_overwrite_contrecord.pl
index 0fd907f152..fe175f5e90 100644
--- a/src/test/recovery/t/026_overwrite_contrecord.pl
+++ b/src/test/recovery/t/026_overwrite_contrecord.pl
@@ -13,7 +13,7 @@ use Test::More;
# Test: Create a physical replica that's missing the last WAL file,
# then restart the primary to create a divergent WAL file and observe
# that the replica replays the "overwrite contrecord" from that new
-# file.
+# file and the standby promotes successfully.
my $node = PostgreSQL::Test::Cluster->new('primary');
$node->init(allows_streaming => 1);
@@ -100,6 +100,9 @@ like(
qr[successfully skipped missing contrecord at],
"found log line in standby");
+# Verify promotion is successful
+$node_standby->safe_psql('postgres', "select pg_promote()");
+
$node->stop;
$node_standby->stop;
--
2.32.0
view thread (11+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: [BUG] Panic due to incorrect missingContrecPtr after promotion
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox