From: David Steele <david@pgbackrest.org>
To: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Improve verification of recovery_target_timeline GUC.
Date: Thu, 23 Jan 2025 14:53:39 +0000 (UTC)
Message-ID: <e5d472c7-e9be-4710-8dc4-ebe721b62cea@pgbackrest.org> (raw)
Hackers,
Currently check_recovery_target_timeline() converts any value that is
not current, latest, or a valid integer to 0. So for example:
recovery_target_timeline = 'currrent'
results in the following error:
FATAL: 22023: recovery target timeline 0 does not exist
Since there is no range checking for uint32 (but there is a cast from
unsigned long) this setting:
recovery_target_timeline = '9999999999'
results in the following error (on 64-bit):
FATAL: 22023: recovery target timeline 1410065407 does not exist
Improve by adding endptr checking to catch conversion errors and add
range checking to exclude values < 2 and greater than UINT_MAX.
I discovered this while testing on Postgres versions < 12 where
recovery_target_timeline = 'current'
sets the target timeline to 'latest', which is a pretty surprising
result. I'm only noting it in case somebody is searching the archives
since the versions in question are EOL. But that odd result made me
wonder if Postgres >= 12 had a similar problem. While it is not quite so
confusing as Postgres < 12 I think it still bears improving.
The tests are probably excessive but I needed something to show that the
verification works as expected.
Regards,
-David
Attachments:timeline-log-v1.patch (text/plain, 1.6K) download
From f14e30b18cde216131bd3e069ee8ecd5da3301b0 Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
Date: Fri, 20 Dec 2024 15:13:59 +0000
Subject: Fix logging for invalid recovery timeline.
If the requested recovery timeline is not reachable the logged checkpoint and
TLI should to be the values read from backup_label when backup_label is
present.
---
src/backend/access/transam/xlogrecovery.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.cindex c6994b78282..99afd01bf38 100644--- a/src/backend/access/transam/xlogrecovery.c+++ b/src/backend/access/transam/xlogrecovery.c@@ -844,13 +844,13 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
* tliSwitchPoint will throw an error if the checkpoint's timeline is
* not in expectedTLEs at all.
*/
- switchpoint = tliSwitchPoint(ControlFile->checkPointCopy.ThisTimeLineID, expectedTLEs, NULL);+ switchpoint = tliSwitchPoint(CheckPointTLI, expectedTLEs, NULL);
ereport(FATAL,
(errmsg("requested timeline %u is not a child of this server's history",
recoveryTargetTLI),
errdetail("Latest checkpoint is at %X/%X on timeline %u, but in the history of the requested timeline, the server forked off from that timeline at %X/%X.",
- LSN_FORMAT_ARGS(ControlFile->checkPoint),- ControlFile->checkPointCopy.ThisTimeLineID,+ LSN_FORMAT_ARGS(CheckPointLoc),+ CheckPointTLI,
LSN_FORMAT_ARGS(switchpoint))));
}
--
2.34.1
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: pgsql-hackers@postgresql.org
Cc: david@pgbackrest.org
Subject: Re: Improve verification of recovery_target_timeline GUC.
In-Reply-To: <e5d472c7-e9be-4710-8dc4-ebe721b62cea@pgbackrest.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox