[PATCH] Allow usage of archive .backup files as backup_label
Hi,
The .backup files written to the archive (if archiving is on) are very
similar to the backup_label that's written/returned by
pg_stop_backup()/pg_backup_stop(), they just have a few extra lines
about the end of backup process that are missing from backup_label.
The parser in xlogrecovery.c however barfs on them because it does not
expect the additional STOP WAL LOCATION on line 2.
The attached makes parsing this line optional, so that one can use those
.backup files in place of backup_label. This is e.g. useful if the
backup_label got lost or the output of pg_stop_backup() was not
captured.
Michael
--
Team Lead PostgreSQL
Project Manager
Tel.: +49 2166 9901-171
Mail: michael.banck@credativ.de
credativ GmbH, HRB M�nchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 M�nchengladbach
Management: Dr. Michael Meskes, Geoff Richardson, Peter Lilley
Our handling of personal data is subject to:
https://www.credativ.de/en/contact/privacy/
Attachments:
0001-Allow-usage-of-archive-.backup-files-as-backup_label.patchtext/x-diff; charset=us-asciiDownload
From 00b1b245f74a0496a4d60cfafff92735dbe73d22 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.banck@credativ.de>
Date: Mon, 22 Aug 2022 16:20:14 +0200
Subject: [PATCH] Allow usage of archive .backup files as backup_label.
This lets the backup_label parser not bail if STOP WAL LOCATION is encountered,
which is the only meaningful difference between an archive .backup file and a
backup_label file, thus allowing to just copy the corresponding .backup file
from the archive as backup_label, in case the backup_label file got lost or was
never recorded.
---
src/backend/access/transam/xlogrecovery.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index a59a0e826b..a95946e391 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1149,6 +1149,7 @@ read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
bool *backupEndRequired, bool *backupFromStandby)
{
char startxlogfilename[MAXFNAMELEN];
+ char stopxlogfilename[MAXFNAMELEN];
TimeLineID tli_from_walseg,
tli_from_file;
FILE *lfp;
@@ -1183,7 +1184,10 @@ read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
/*
* Read and parse the START WAL LOCATION and CHECKPOINT lines (this code
* is pretty crude, but we are not expecting any variability in the file
- * format).
+ * format). Also allow STOP WAL LOCATION to be in the file. This line does
+ * not appear in backup_label, but it is written to the corresponding
+ * .backup file and allows users to rename or copy that file to
+ * backup_label without further editing.
*/
if (fscanf(lfp, "START WAL LOCATION: %X/%X (file %08X%16s)%c",
&hi, &lo, &tli_from_walseg, startxlogfilename, &ch) != 5 || ch != '\n')
@@ -1192,6 +1196,11 @@ read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
RedoStartLSN = ((uint64) hi) << 32 | lo;
RedoStartTLI = tli_from_walseg;
+ if (fscanf(lfp, "STOP WAL LOCATION: %X/%X (file %*08X%16s)%c",
+ &hi, &lo, stopxlogfilename, &ch) == 4)
+ ereport(DEBUG1,
+ (errmsg_internal("stop wal location %X/%X in file \"%s\"",
+ hi, lo, BACKUP_LABEL_FILE)));
if (fscanf(lfp, "CHECKPOINT LOCATION: %X/%X%c",
&hi, &lo, &ch) != 3 || ch != '\n')
ereport(FATAL,
--
2.30.2
On Mon, Aug 22, 2022 at 05:16:58PM +0200, Michael Banck wrote:
The .backup files written to the archive (if archiving is on) are very
similar to the backup_label that's written/returned by
pg_stop_backup()/pg_backup_stop(), they just have a few extra lines
about the end of backup process that are missing from backup_label.
Historically, there is "STOP WAL LOCATION" after "START WAL LOCATION",
and "STOP TIME"/"STOP TIMELINE" at the end.
The parser in xlogrecovery.c however barfs on them because it does not
expect the additional STOP WAL LOCATION on line 2.
Hm, no. I don't think that I'd want to expand the use of the backup
history file in the context of recovery, so as we are free to add any
extra information into it if necessary without impacting the
compatibility of the recovery code. This file is primarily here for
debugging, so I'd rather let it be used only for this purpose.
Opinions of others are welcome, of course.
--
Michael
On Tue, 2022-10-18 at 10:55 +0900, Michael Paquier wrote:
On Mon, Aug 22, 2022 at 05:16:58PM +0200, Michael Banck wrote:
The .backup files written to the archive (if archiving is on) are very
similar to the backup_label that's written/returned by
pg_stop_backup()/pg_backup_stop(), they just have a few extra lines
about the end of backup process that are missing from backup_label.Historically, there is "STOP WAL LOCATION" after "START WAL LOCATION",
and "STOP TIME"/"STOP TIMELINE" at the end.The parser in xlogrecovery.c however barfs on them because it does not
expect the additional STOP WAL LOCATION on line 2.Hm, no. I don't think that I'd want to expand the use of the backup
history file in the context of recovery, so as we are free to add any
extra information into it if necessary without impacting the
compatibility of the recovery code. This file is primarily here for
debugging, so I'd rather let it be used only for this purpose.
Opinions of others are welcome, of course.
I tend to agree with you. It is easy to break PostgreSQL by manipulating
or removing "backup_label", and copying a file from the WAL archive and
renaming it to "backup_label" sounds like a footgun of the first order.
There is nothing that prevents you from copying the wrong file.
Such practices should not be encouraged.
Anybody who knows enough about PostgreSQL to be sure that what they are
doing is correct should be smart enough to know how to edit the copied file.
Yours,
Laurenz Albe
On Tue, Oct 18, 2022 at 04:55:46AM +0200, Laurenz Albe wrote:
I tend to agree with you. It is easy to break PostgreSQL by manipulating
or removing "backup_label", and copying a file from the WAL archive and
renaming it to "backup_label" sounds like a footgun of the first order.
There is nothing that prevents you from copying the wrong file.
Such practices should not be encouraged.Anybody who knows enough about PostgreSQL to be sure that what they are
doing is correct should be smart enough to know how to edit the copied file.
A few weeks after, still the same thoughts on the matter, so please
note that I have marked that as rejected in the CF app. If somebody
wants to offer more arguments for this thread, of course please feel
free.
--
Michael