public inbox for [email protected]  
help / color / mirror / Atom feed
From: David Steele <[email protected]>
To: Thomas Munro <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Stephen Frost <[email protected]>
Subject: Re: The danger of deleting backup_label
Date: Sat, 14 Oct 2023 11:30:55 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<CA+hUKGLok_wrosp8Q6DA+mkhtWyQSEwPSKP=2p5bnLZC=FGpaQ@mail.gmail.com>
	<[email protected]>

On 10/12/23 10:19, David Steele wrote:
> On 10/11/23 18:10, Thomas Munro wrote:
> 
>> As Stephen mentioned[1], we could perhaps also complain if both backup
>> label and control file exist, and then hint that the user should
>> remove the *control file* (not the backup label!).  I had originally
>> suggested we would just overwrite the control file, but by explicitly
>> complaining about it we would also bring the matter to tool/script
>> authors' attention, ie that they shouldn't be backing that file up, or
>> should be removing it in a later step if they copy everything.  He
>> also mentions that there doesn't seem to be anything stopping us from
>> back-patching changes to the backup label contents if we go this way.
>> I don't have a strong opinion on that and we could leave the question
>> for later.
> 
> I'm worried about the possibility of back patching this unless the 
> solution comes out to be simpler than I think and that rarely comes to 
> pass. Surely throwing errors on something that is currently valid (i.e. 
> backup_label and pg_control both present).
> 
> But perhaps there is a simpler, acceptable solution we could back patch 
> (transparent to all parties except Postgres) and then a more advanced 
> solution we could go forward with.
> 
> I guess I had better get busy on this.

Attached is a very POC attempt at something along these lines that could 
be back patched. I stopped when I realized excluding pg_control from the 
backup is a necessity to make this work (else we still could end up with 
a torn copy of pg_control even if there is a good copy elsewhere). To 
enumerate the back patch issues as I see them:

1) We still need a copy of pg_control in order to get Postgres to start 
and that copy might be torn (pretty much where we are now). We can 
handle this easily in pg_basebackup but most backup software will not be 
able to do so. In my opinion teaching Postgres to start without 
pg_control is too big a change to possibly back patch.

2) We need to deal with backups made with a prior *minor* version that 
did not include pg_control in the backup_label. Doable, but...

3) We need to move backup_label to the end of the main pg_basebackup 
tar, which could cause unforeseen breakage for tools.

4) This patch is less efficient for backups taken from standby because 
it will overwrite pg_control on restart and force replay back to the 
original MinRecoveryPoint.

5) We still need a solution for exclusive backup (still valid in PG <= 
14). Doable but it would still have the weakness of 1.

All of this is fixable in HEAD, but seems incredibly dangerous to back 
patch. Even so, I have attached the patch in case somebody sees an 
opportunity that I do not.

Regards,
-David
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c0e4ca50899..e4a4478af75 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -74,6 +74,7 @@
 #include "pg_trace.h"
 #include "pgstat.h"
 #include "port/atomics.h"
+#include "port/pg_crc32c.h"
 #include "port/pg_iovec.h"
 #include "postmaster/bgwriter.h"
 #include "postmaster/startup.h"
@@ -8691,6 +8692,21 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 						 "and should not be used. "
 						 "Try taking another online backup.")));
 
+	/*
+	 * Create a copy of control data to be stored in the backup label.
+	 * Recalculate the CRC so recovery can validate the contents but do not
+	 * bother with the timestamp since that will be applied before it is
+	 * written by recovery.
+	 */
+	LWLockAcquire(ControlFileLock, LW_SHARED);
+	state->controlFile = *ControlFile;
+	LWLockRelease(ControlFileLock);
+
+	INIT_CRC32C(state->controlFile.crc);
+	COMP_CRC32C(state->controlFile.crc, (char *)&state->controlFile,
+			    offsetof(ControlFileData, crc));
+	FIN_CRC32C(state->controlFile.crc);
+
 	/*
 	 * During recovery, we don't write an end-of-backup record. We assume that
 	 * pg_control was backed up last and its minimum recovery point can be
@@ -8741,11 +8757,8 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 							 "Enable full_page_writes and run CHECKPOINT on the primary, "
 							 "and then try an online backup again.")));
 
-
-		LWLockAcquire(ControlFileLock, LW_SHARED);
-		state->stoppoint = ControlFile->minRecoveryPoint;
-		state->stoptli = ControlFile->minRecoveryPointTLI;
-		LWLockRelease(ControlFileLock);
+		state->stoppoint = state->controlFile.minRecoveryPoint;
+		state->stoptli = state->controlFile.minRecoveryPointTLI;
 	}
 	else
 	{
diff --git a/src/backend/access/transam/xlogbackup.c b/src/backend/access/transam/xlogbackup.c
index 21d68133ae1..79559d78acb 100644
--- a/src/backend/access/transam/xlogbackup.c
+++ b/src/backend/access/transam/xlogbackup.c
@@ -16,6 +16,7 @@
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogbackup.h"
+#include "common/base64.h"
 
 /*
  * Build contents for backup_label or backup history file.
@@ -76,6 +77,17 @@ build_backup_content(BackupState *state, bool ishistoryfile)
 		appendStringInfo(result, "STOP TIME: %s\n", stopstrfbuf);
 		appendStringInfo(result, "STOP TIMELINE: %u\n", state->stoptli);
 	}
+	/* Include a copy of control data */
+	else
+	{
+		char controlbuf[((sizeof(ControlFileData) + 2) / 3 * 4) + 1];
+
+		pg_b64_encode((char *)&state->controlFile, sizeof(ControlFileData),
+					  controlbuf, sizeof(controlbuf) - 1);
+		controlbuf[sizeof(controlbuf) - 1] = '\0';
+
+		appendStringInfo(result, "CONTROL DATA: %s\n", controlbuf);
+	}
 
 	data = result->data;
 	pfree(result);
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index becc2bda62e..1abd9494bf0 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -43,6 +43,7 @@
 #include "backup/basebackup.h"
 #include "catalog/pg_control.h"
 #include "commands/tablespace.h"
+#include "common/base64.h"
 #include "common/file_utils.h"
 #include "miscadmin.h"
 #include "pgstat.h"
@@ -390,7 +391,8 @@ static void readRecoverySignalFile(void);
 static void validateRecoveryParameters(void);
 static bool read_backup_label(XLogRecPtr *checkPointLoc,
 							  TimeLineID *backupLabelTLI,
-							  bool *backupEndRequired, bool *backupFromStandby);
+							  bool *backupEndRequired, bool *backupFromStandby,
+							  ControlFileData *ControlFile);
 static bool read_tablespace_map(List **tablespaces);
 
 static void xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI);
@@ -610,7 +612,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
 	primary_image_masked = (char *) palloc(BLCKSZ);
 
 	if (read_backup_label(&CheckPointLoc, &CheckPointTLI, &backupEndRequired,
-						  &backupFromStandby))
+						  &backupFromStandby, ControlFile))
 	{
 		List	   *tablespaces = NIL;
 
@@ -1167,7 +1169,8 @@ validateRecoveryParameters(void)
  */
 static bool
 read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
-				  bool *backupEndRequired, bool *backupFromStandby)
+				  bool *backupEndRequired, bool *backupFromStandby,
+				  ControlFileData *ControlFile)
 {
 	char		startxlogfilename[MAXFNAMELEN];
 	TimeLineID	tli_from_walseg,
@@ -1180,6 +1183,7 @@ read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
 	char		backuptime[128];
 	uint32		hi,
 				lo;
+	char 		controlB64Buf[684 + 1];
 
 	/* suppress possible uninitialized-variable warnings */
 	*checkPointLoc = InvalidXLogRecPtr;
@@ -1285,6 +1289,51 @@ read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
 								 tli_from_file, BACKUP_LABEL_FILE)));
 	}
 
+	/*
+	 * Read control data to be used to create pg_control. Control data may not
+	 * exist if the backup was made with an older version, in which case the
+	 * control file read from disk will be used.
+	 */
+	if (fscanf(lfp, "CONTROL DATA: %684s\n", controlB64Buf) == 1)
+	{
+		ControlFileData controlBuf;
+		pg_crc32c 		crc;
+
+		/* Check that the base64 data is the correct length */
+		if (strlen(controlB64Buf) != (sizeof(ControlFileData) + 2) / 3 * 4)
+			ereport(FATAL,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE),
+					 errdetail("Control data expected %zu base64 characters.",
+							   (sizeof(ControlFileData) + 2) / 3 * 4)));
+
+		/* Decode control file */
+		if (pg_b64_decode(controlB64Buf, strlen(controlB64Buf), (char *)&controlBuf,
+		                  sizeof(ControlFileData)) == -1)
+			ereport(FATAL,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE),
+					 errdetail("Control data contains invalid base64 encoding.")));
+
+		/* CRC check on stored control file */
+		INIT_CRC32C(crc);
+		COMP_CRC32C(crc, (char *)&controlBuf, offsetof(ControlFileData, crc));
+		FIN_CRC32C(crc);
+
+		if (!EQ_CRC32C(crc, controlBuf.crc))
+			ereport(FATAL,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE),
+					 errdetail("Control data contains invalid CRC.")));
+
+		ereport(DEBUG1,
+				(errmsg_internal("backup control data in file \"%s\"",
+								 BACKUP_LABEL_FILE)));
+
+		/* Copy control data */
+		memcpy(ControlFile, &controlBuf, sizeof(ControlFileData));
+	}
+
 	if (ferror(lfp) || FreeFile(lfp))
 		ereport(FATAL,
 				(errcode_for_file_access(),
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 7d025bcf382..f49dabf63ef 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -330,13 +330,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 
 				bbsink_begin_archive(sink, "base.tar");
 
-				/* In the main tar, include the backup_label first... */
-				backup_label = build_backup_content(backup_state, false);
-				sendFileWithContent(sink, BACKUP_LABEL_FILE,
-									backup_label, &manifest);
-				pfree(backup_label);
-
-				/* Then the tablespace_map file, if required... */
+				/* Send the tablespace_map file, if required... */
 				if (opt->sendtblspcmapfile)
 				{
 					sendFileWithContent(sink, TABLESPACE_MAP,
@@ -348,7 +342,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 				sendDir(sink, ".", 1, false, state.tablespaces,
 						sendtblspclinks, &manifest, NULL);
 
-				/* ... and pg_control after everything else. */
+				/* ... and pg_control after everything but backup_label */
 				if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0)
 					ereport(ERROR,
 							(errcode_for_file_access(),
@@ -356,6 +350,16 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 									XLOG_CONTROL_FILE)));
 				sendFile(sink, XLOG_CONTROL_FILE, XLOG_CONTROL_FILE, &statbuf,
 						 false, InvalidOid, &manifest, NULL);
+
+				/* End the backup before sending backup_label */
+				basebackup_progress_wait_wal_archive(&state);
+				do_pg_backup_stop(backup_state, !opt->nowait);
+
+				/* Last in the main tar, include backup_label */
+				backup_label = build_backup_content(backup_state, false);
+				sendFileWithContent(sink, BACKUP_LABEL_FILE,
+									backup_label, &manifest);
+				pfree(backup_label);
 			}
 			else
 			{
@@ -389,9 +393,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 			}
 		}
 
-		basebackup_progress_wait_wal_archive(&state);
-		do_pg_backup_stop(backup_state, !opt->nowait);
-
 		endptr = backup_state->stoppoint;
 		endtli = backup_state->stoptli;
 
diff --git a/src/include/access/xlogbackup.h b/src/include/access/xlogbackup.h
index 1611358137b..bd53405670e 100644
--- a/src/include/access/xlogbackup.h
+++ b/src/include/access/xlogbackup.h
@@ -15,6 +15,7 @@
 #define XLOG_BACKUP_H
 
 #include "access/xlogdefs.h"
+#include "catalog/pg_control.h"
 #include "pgtime.h"
 
 /* Structure to hold backup state. */
@@ -33,6 +34,7 @@ typedef struct BackupState
 	XLogRecPtr	stoppoint;		/* backup stop WAL location */
 	TimeLineID	stoptli;		/* backup stop TLI */
 	pg_time_t	stoptime;		/* backup stop time */
+	ControlFileData controlFile;	/* Copy of control data */
 } BackupState;
 
 extern char *build_backup_content(BackupState *state,


Attachments:

  [text/plain] v01-pgcontrol-in-backup-label.patch (9.3K, ../[email protected]/2-v01-pgcontrol-in-backup-label.patch)
  download | inline diff:
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c0e4ca50899..e4a4478af75 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -74,6 +74,7 @@
 #include "pg_trace.h"
 #include "pgstat.h"
 #include "port/atomics.h"
+#include "port/pg_crc32c.h"
 #include "port/pg_iovec.h"
 #include "postmaster/bgwriter.h"
 #include "postmaster/startup.h"
@@ -8691,6 +8692,21 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 						 "and should not be used. "
 						 "Try taking another online backup.")));
 
+	/*
+	 * Create a copy of control data to be stored in the backup label.
+	 * Recalculate the CRC so recovery can validate the contents but do not
+	 * bother with the timestamp since that will be applied before it is
+	 * written by recovery.
+	 */
+	LWLockAcquire(ControlFileLock, LW_SHARED);
+	state->controlFile = *ControlFile;
+	LWLockRelease(ControlFileLock);
+
+	INIT_CRC32C(state->controlFile.crc);
+	COMP_CRC32C(state->controlFile.crc, (char *)&state->controlFile,
+			    offsetof(ControlFileData, crc));
+	FIN_CRC32C(state->controlFile.crc);
+
 	/*
 	 * During recovery, we don't write an end-of-backup record. We assume that
 	 * pg_control was backed up last and its minimum recovery point can be
@@ -8741,11 +8757,8 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 							 "Enable full_page_writes and run CHECKPOINT on the primary, "
 							 "and then try an online backup again.")));
 
-
-		LWLockAcquire(ControlFileLock, LW_SHARED);
-		state->stoppoint = ControlFile->minRecoveryPoint;
-		state->stoptli = ControlFile->minRecoveryPointTLI;
-		LWLockRelease(ControlFileLock);
+		state->stoppoint = state->controlFile.minRecoveryPoint;
+		state->stoptli = state->controlFile.minRecoveryPointTLI;
 	}
 	else
 	{
diff --git a/src/backend/access/transam/xlogbackup.c b/src/backend/access/transam/xlogbackup.c
index 21d68133ae1..79559d78acb 100644
--- a/src/backend/access/transam/xlogbackup.c
+++ b/src/backend/access/transam/xlogbackup.c
@@ -16,6 +16,7 @@
 #include "access/xlog.h"
 #include "access/xlog_internal.h"
 #include "access/xlogbackup.h"
+#include "common/base64.h"
 
 /*
  * Build contents for backup_label or backup history file.
@@ -76,6 +77,17 @@ build_backup_content(BackupState *state, bool ishistoryfile)
 		appendStringInfo(result, "STOP TIME: %s\n", stopstrfbuf);
 		appendStringInfo(result, "STOP TIMELINE: %u\n", state->stoptli);
 	}
+	/* Include a copy of control data */
+	else
+	{
+		char controlbuf[((sizeof(ControlFileData) + 2) / 3 * 4) + 1];
+
+		pg_b64_encode((char *)&state->controlFile, sizeof(ControlFileData),
+					  controlbuf, sizeof(controlbuf) - 1);
+		controlbuf[sizeof(controlbuf) - 1] = '\0';
+
+		appendStringInfo(result, "CONTROL DATA: %s\n", controlbuf);
+	}
 
 	data = result->data;
 	pfree(result);
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index becc2bda62e..1abd9494bf0 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -43,6 +43,7 @@
 #include "backup/basebackup.h"
 #include "catalog/pg_control.h"
 #include "commands/tablespace.h"
+#include "common/base64.h"
 #include "common/file_utils.h"
 #include "miscadmin.h"
 #include "pgstat.h"
@@ -390,7 +391,8 @@ static void readRecoverySignalFile(void);
 static void validateRecoveryParameters(void);
 static bool read_backup_label(XLogRecPtr *checkPointLoc,
 							  TimeLineID *backupLabelTLI,
-							  bool *backupEndRequired, bool *backupFromStandby);
+							  bool *backupEndRequired, bool *backupFromStandby,
+							  ControlFileData *ControlFile);
 static bool read_tablespace_map(List **tablespaces);
 
 static void xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI);
@@ -610,7 +612,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
 	primary_image_masked = (char *) palloc(BLCKSZ);
 
 	if (read_backup_label(&CheckPointLoc, &CheckPointTLI, &backupEndRequired,
-						  &backupFromStandby))
+						  &backupFromStandby, ControlFile))
 	{
 		List	   *tablespaces = NIL;
 
@@ -1167,7 +1169,8 @@ validateRecoveryParameters(void)
  */
 static bool
 read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
-				  bool *backupEndRequired, bool *backupFromStandby)
+				  bool *backupEndRequired, bool *backupFromStandby,
+				  ControlFileData *ControlFile)
 {
 	char		startxlogfilename[MAXFNAMELEN];
 	TimeLineID	tli_from_walseg,
@@ -1180,6 +1183,7 @@ read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
 	char		backuptime[128];
 	uint32		hi,
 				lo;
+	char 		controlB64Buf[684 + 1];
 
 	/* suppress possible uninitialized-variable warnings */
 	*checkPointLoc = InvalidXLogRecPtr;
@@ -1285,6 +1289,51 @@ read_backup_label(XLogRecPtr *checkPointLoc, TimeLineID *backupLabelTLI,
 								 tli_from_file, BACKUP_LABEL_FILE)));
 	}
 
+	/*
+	 * Read control data to be used to create pg_control. Control data may not
+	 * exist if the backup was made with an older version, in which case the
+	 * control file read from disk will be used.
+	 */
+	if (fscanf(lfp, "CONTROL DATA: %684s\n", controlB64Buf) == 1)
+	{
+		ControlFileData controlBuf;
+		pg_crc32c 		crc;
+
+		/* Check that the base64 data is the correct length */
+		if (strlen(controlB64Buf) != (sizeof(ControlFileData) + 2) / 3 * 4)
+			ereport(FATAL,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE),
+					 errdetail("Control data expected %zu base64 characters.",
+							   (sizeof(ControlFileData) + 2) / 3 * 4)));
+
+		/* Decode control file */
+		if (pg_b64_decode(controlB64Buf, strlen(controlB64Buf), (char *)&controlBuf,
+		                  sizeof(ControlFileData)) == -1)
+			ereport(FATAL,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE),
+					 errdetail("Control data contains invalid base64 encoding.")));
+
+		/* CRC check on stored control file */
+		INIT_CRC32C(crc);
+		COMP_CRC32C(crc, (char *)&controlBuf, offsetof(ControlFileData, crc));
+		FIN_CRC32C(crc);
+
+		if (!EQ_CRC32C(crc, controlBuf.crc))
+			ereport(FATAL,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE),
+					 errdetail("Control data contains invalid CRC.")));
+
+		ereport(DEBUG1,
+				(errmsg_internal("backup control data in file \"%s\"",
+								 BACKUP_LABEL_FILE)));
+
+		/* Copy control data */
+		memcpy(ControlFile, &controlBuf, sizeof(ControlFileData));
+	}
+
 	if (ferror(lfp) || FreeFile(lfp))
 		ereport(FATAL,
 				(errcode_for_file_access(),
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 7d025bcf382..f49dabf63ef 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -330,13 +330,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 
 				bbsink_begin_archive(sink, "base.tar");
 
-				/* In the main tar, include the backup_label first... */
-				backup_label = build_backup_content(backup_state, false);
-				sendFileWithContent(sink, BACKUP_LABEL_FILE,
-									backup_label, &manifest);
-				pfree(backup_label);
-
-				/* Then the tablespace_map file, if required... */
+				/* Send the tablespace_map file, if required... */
 				if (opt->sendtblspcmapfile)
 				{
 					sendFileWithContent(sink, TABLESPACE_MAP,
@@ -348,7 +342,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 				sendDir(sink, ".", 1, false, state.tablespaces,
 						sendtblspclinks, &manifest, NULL);
 
-				/* ... and pg_control after everything else. */
+				/* ... and pg_control after everything but backup_label */
 				if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0)
 					ereport(ERROR,
 							(errcode_for_file_access(),
@@ -356,6 +350,16 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 									XLOG_CONTROL_FILE)));
 				sendFile(sink, XLOG_CONTROL_FILE, XLOG_CONTROL_FILE, &statbuf,
 						 false, InvalidOid, &manifest, NULL);
+
+				/* End the backup before sending backup_label */
+				basebackup_progress_wait_wal_archive(&state);
+				do_pg_backup_stop(backup_state, !opt->nowait);
+
+				/* Last in the main tar, include backup_label */
+				backup_label = build_backup_content(backup_state, false);
+				sendFileWithContent(sink, BACKUP_LABEL_FILE,
+									backup_label, &manifest);
+				pfree(backup_label);
 			}
 			else
 			{
@@ -389,9 +393,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
 			}
 		}
 
-		basebackup_progress_wait_wal_archive(&state);
-		do_pg_backup_stop(backup_state, !opt->nowait);
-
 		endptr = backup_state->stoppoint;
 		endtli = backup_state->stoptli;
 
diff --git a/src/include/access/xlogbackup.h b/src/include/access/xlogbackup.h
index 1611358137b..bd53405670e 100644
--- a/src/include/access/xlogbackup.h
+++ b/src/include/access/xlogbackup.h
@@ -15,6 +15,7 @@
 #define XLOG_BACKUP_H
 
 #include "access/xlogdefs.h"
+#include "catalog/pg_control.h"
 #include "pgtime.h"
 
 /* Structure to hold backup state. */
@@ -33,6 +34,7 @@ typedef struct BackupState
 	XLogRecPtr	stoppoint;		/* backup stop WAL location */
 	TimeLineID	stoptli;		/* backup stop TLI */
 	pg_time_t	stoptime;		/* backup stop time */
+	ControlFileData controlFile;	/* Copy of control data */
 } BackupState;
 
 extern char *build_backup_content(BackupState *state,


view thread (18+ 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], [email protected]
  Subject: Re: The danger of deleting backup_label
  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