Correct error message for end-of-recovery record TLI

Started by Amul Sulabout 4 years ago10 messages
#1Amul Sul
sulamul@gmail.com
1 attachment(s)

Hi,

In xlog_redo, for end-of-recovery case error message describes the
record as a checkpoint record which seems to be incorrect; the
attached patch corrects that.

--
Regards,
Amul Sul
EDB: http://www.enterprisedb.com

Attachments:

correct_xlog_redo_errmsg.patchapplication/x-patch; name=correct_xlog_redo_errmsg.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f547efd2944..77894b5bb2e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10460,7 +10460,7 @@ xlog_redo(XLogReaderState *record)
 		/* TLI should not change in an on-line checkpoint */
 		if (checkPoint.ThisTimeLineID != ThisTimeLineID)
 			ereport(PANIC,
-					(errmsg("unexpected timeline ID %u (should be %u) in checkpoint record",
+					(errmsg("unexpected timeline ID %u (should be %u) in end-of-recovery record",
 							checkPoint.ThisTimeLineID, ThisTimeLineID)));
 
 		RecoveryRestartPoint(&checkPoint);
#2Robert Haas
robertmhaas@gmail.com
In reply to: Amul Sul (#1)
Re: Correct error message for end-of-recovery record TLI

On Thu, Oct 28, 2021 at 8:59 AM Amul Sul <sulamul@gmail.com> wrote:

In xlog_redo, for end-of-recovery case error message describes the
record as a checkpoint record which seems to be incorrect; the
attached patch corrects that.

The analysis and patch appear correct to me.

--
Robert Haas
EDB: http://www.enterprisedb.com

#3Bossart, Nathan
bossartn@amazon.com
In reply to: Robert Haas (#2)
Re: Correct error message for end-of-recovery record TLI

On 10/28/21, 12:23 PM, "Robert Haas" <robertmhaas@gmail.com> wrote:

On Thu, Oct 28, 2021 at 8:59 AM Amul Sul <sulamul@gmail.com> wrote:

In xlog_redo, for end-of-recovery case error message describes the
record as a checkpoint record which seems to be incorrect; the
attached patch corrects that.

The analysis and patch appear correct to me.

When I apply the patch, it changes the PANIC message in the
XLOG_CHECKPOINT_ONLINE section, not the XLOG_END_OF_RECOVERY one.

Nathan

#4Bossart, Nathan
bossartn@amazon.com
In reply to: Robert Haas (#2)
Re: Correct error message for end-of-recovery record TLI

On 10/28/21, 12:52 PM, "Bossart, Nathan" <bossartn@amazon.com> wrote:

On 10/28/21, 12:23 PM, "Robert Haas" <robertmhaas@gmail.com> wrote:

On Thu, Oct 28, 2021 at 8:59 AM Amul Sul <sulamul@gmail.com> wrote:

In xlog_redo, for end-of-recovery case error message describes the
record as a checkpoint record which seems to be incorrect; the
attached patch corrects that.

The analysis and patch appear correct to me.

When I apply the patch, it changes the PANIC message in the
XLOG_CHECKPOINT_ONLINE section, not the XLOG_END_OF_RECOVERY one.

But other than that, the change seems reasonable to me.

Nathan

#5Robert Haas
robertmhaas@gmail.com
In reply to: Bossart, Nathan (#3)
Re: Correct error message for end-of-recovery record TLI

On Thu, Oct 28, 2021 at 3:52 PM Bossart, Nathan <bossartn@amazon.com> wrote:

When I apply the patch, it changes the PANIC message in the
XLOG_CHECKPOINT_ONLINE section, not the XLOG_END_OF_RECOVERY one.

Well that's a good point. *facepalm*

--
Robert Haas
EDB: http://www.enterprisedb.com

#6Amul Sul
sulamul@gmail.com
In reply to: Robert Haas (#5)
1 attachment(s)
Re: Correct error message for end-of-recovery record TLI

On Fri, Oct 29, 2021 at 2:36 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Oct 28, 2021 at 3:52 PM Bossart, Nathan <bossartn@amazon.com> wrote:

When I apply the patch, it changes the PANIC message in the
XLOG_CHECKPOINT_ONLINE section, not the XLOG_END_OF_RECOVERY one.

Well that's a good point. *facepalm*

Oops... :|

Corrected in the attached version.

Regards,
Amul

Attachments:

v2_correct_xlog_redo_errmsg.patchapplication/octet-stream; name=v2_correct_xlog_redo_errmsg.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f547efd2944..8ba6406b489 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10490,7 +10490,7 @@ xlog_redo(XLogReaderState *record)
 		 */
 		if (xlrec.ThisTimeLineID != ThisTimeLineID)
 			ereport(PANIC,
-					(errmsg("unexpected timeline ID %u (should be %u) in checkpoint record",
+					(errmsg("unexpected timeline ID %u (should be %u) in end-of-recovery record",
 							xlrec.ThisTimeLineID, ThisTimeLineID)));
 	}
 	else if (info == XLOG_NOOP)
#7Bossart, Nathan
bossartn@amazon.com
In reply to: Amul Sul (#6)
1 attachment(s)
Re: Correct error message for end-of-recovery record TLI

On 10/28/21, 9:15 PM, "Amul Sul" <sulamul@gmail.com> wrote:

On Fri, Oct 29, 2021 at 2:36 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Oct 28, 2021 at 3:52 PM Bossart, Nathan <bossartn@amazon.com> wrote:

When I apply the patch, it changes the PANIC message in the
XLOG_CHECKPOINT_ONLINE section, not the XLOG_END_OF_RECOVERY one.

Well that's a good point. *facepalm*

Oops... :|

Corrected in the attached version.

The patch no longer applied, so I went ahead and rebased it.

Nathan

Attachments:

v3-0001-correct-end-of-recovery-error-message.patchapplication/octet-stream; name=v3-0001-correct-end-of-recovery-error-message.patchDownload
From 46ee9af69a0fcc730bce3f3959fb903dbe059732 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Wed, 1 Dec 2021 19:06:35 +0000
Subject: [PATCH v3 1/1] correct end-of-recovery error message

---
 src/backend/access/transam/xlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index d894af310a..845ddee647 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10540,7 +10540,7 @@ xlog_redo(XLogReaderState *record)
 		 */
 		if (xlrec.ThisTimeLineID != replayTLI)
 			ereport(PANIC,
-					(errmsg("unexpected timeline ID %u (should be %u) in checkpoint record",
+					(errmsg("unexpected timeline ID %u (should be %u) in end-of-recovery record",
 							xlrec.ThisTimeLineID, replayTLI)));
 	}
 	else if (info == XLOG_NOOP)
-- 
2.16.6

#8Michael Paquier
michael@paquier.xyz
In reply to: Bossart, Nathan (#7)
Re: Correct error message for end-of-recovery record TLI

On Wed, Dec 01, 2021 at 07:09:34PM +0000, Bossart, Nathan wrote:

The patch no longer applied, so I went ahead and rebased it.

This was on the CF stack for some time, so applied. I have also
changed the messages produced for the shutdown and online checkpoint
records as they used the same messages so as one can get more
context depending on the record types.
--
Michael

#9Amul Sul
sulamul@gmail.com
In reply to: Michael Paquier (#8)
Re: Correct error message for end-of-recovery record TLI

On Tue, Jan 25, 2022 at 10:08 AM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Dec 01, 2021 at 07:09:34PM +0000, Bossart, Nathan wrote:

The patch no longer applied, so I went ahead and rebased it.

This was on the CF stack for some time, so applied. I have also
changed the messages produced for the shutdown and online checkpoint
records as they used the same messages so as one can get more
context depending on the record types.

A ton of thanks for the improvement & the commit.

Regards,
Amul

#10Bossart, Nathan
bossartn@amazon.com
In reply to: Amul Sul (#9)
Re: Correct error message for end-of-recovery record TLI

On 1/24/22, 8:42 PM, "Amul Sul" <sulamul@gmail.com> wrote:

On Tue, Jan 25, 2022 at 10:08 AM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Dec 01, 2021 at 07:09:34PM +0000, Bossart, Nathan wrote:

The patch no longer applied, so I went ahead and rebased it.

This was on the CF stack for some time, so applied. I have also
changed the messages produced for the shutdown and online checkpoint
records as they used the same messages so as one can get more
context depending on the record types.

A ton of thanks for the improvement & the commit.

+1, thanks!

Nathan