On 3/18/26 11:53, Michael Paquier wrote:
On Wed, Mar 18, 2026 at 04:05:24AM +0000, David Steele wrote:
On 3/18/26 08:43, Michael Paquier wrote:
I was wondering if we should have an assertion at least to cross-check
that the contents we store in shared memory never go out-of-sync with
the on-disk contents, in the shape of a USE_ASSERT_CHECKING block that
calls get_controlfile() and memcmp()'s the contents between shmem and
the on-disk file, while the LWLock is taken. We ship the control file
last on purpose, one reason being backups taken from standbys, so that
may be sensible to do.
As far as I can see this should always be true -- I audited all the
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE)
sections and the file is always saved once if is updated. Let me see if I
can add this check without too much pain, e.g. an additional parameter.
This matches with my reads of the code. The attached check, that can
be applied on top of your patches, passes under check-world.
Looks good to me. I have integrated it into the attached patches. I just
changed it to look at our copy instead of the global ControlData since
that is the one we are going to save.
Using the pg_control copy from pg_backup_stop() is entirely optional and
nothing is broken if vendors decide not to use it. This can be demonstrated
by applying the 01 patch without 02. In that case the tests in
042_low_level_backup continue to run. You can also apply 01 and 02 and
revert the test changes in 042_low_level_backup and that works, too.
FWIW, after a second look I am actually wondering if 0002 is safe at
all. The contents of the control file are fetched after we are done
with do_pg_backup_stop(), and there could be a bunch of activity that
happens between the end of do_pg_backup_stop() and the
backup_control_file() call, where the control file could be updated
more and interfere with the recovery startup for some of its fields?
GUC parameter updates that may touch the control file are one thing
popping into mind.
You are correct -- the copy of pg_control needs to happen before
do_pg_backup_stop(). An older version of this patch saved pg_control in
backup_state which made the prior location safe. However, I missed
moving this code when I moved pg_control out of backup_state. Code
review to the rescue.
I believe I have addressed all current review comments in the attached
patches. I tested Postgres crashing right after pg_control is updated
but before backup_label is renamed. It worked as expected on restart. I
also manually removed backup_label before restarting and that worked as
well. I wrote a comment documenting all that.
One thing I could do is note in the documentation that it is not
strictly necessary to get pg_control from pg_backup_stop(). Right now it
sounds like copying from disk is no longer an option -- but it is if you
are willing to accept the possibility of pg_control being torn. But I'd
hope most well-maintained backup software is taking care of that by now.
The problem with caveats in the docs is it can lead to confusion and
getting pg_control from pg_backup_stop() is just a better idea in general.
Thoughts?
Regards,
-David
From 8694c31dee28606448edfa5b0375fad8a7d619bd Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Wed, 18 Mar 2026 07:11:13 +0000
Subject: Add pg_control flag to prevent recovery without backup_label.
Harden recovery by adding a flag to pg_control to indicate that backup_label is
required. This prevents the user from deleting backup_label resulting in an
inconsistent recovery.
Another advantage is that the copy of pg_control used by pg_basebackup is
guaranteed not to be torn.
This functionality is limited to pg_basebackup (or any software comfortable
with modifying pg_control).
Control and catalog version bumps are required.
---
doc/src/sgml/func/func-info.sgml | 5 ++++
src/backend/access/transam/xlog.c | 36 +++++++++++++++++++++++
src/backend/access/transam/xlogrecovery.c | 19 +++++++++++-
src/backend/backup/basebackup.c | 15 ++++------
src/backend/utils/misc/pg_controldata.c | 7 +++--
src/bin/pg_controldata/pg_controldata.c | 2 ++
src/bin/pg_resetwal/pg_resetwal.c | 1 +
src/bin/pg_rewind/pg_rewind.c | 1 +
src/include/access/xlog.h | 1 +
src/include/catalog/pg_control.h | 4 +++
src/include/catalog/pg_proc.dat | 6 ++--
src/test/recovery/t/002_archiving.pl | 20 +++++++++++++
12 files changed, 102 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml
index 5b5f1f3c5df..b1b49ce7b07 100644
--- a/doc/src/sgml/func/func-info.sgml
+++ b/doc/src/sgml/func/func-info.sgml
@@ -3661,6 +3661,11 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres}
<entry><type>boolean</type></entry>
</row>
+ <row>
+ <entry><structfield>backup_label_required</structfield></entry>
+ <entry><type>boolean</type></entry>
+ </row>
+
</tbody>
</tgroup>
</table>
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index f5c9a34374d..87ee245e3f4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9592,6 +9592,42 @@ do_pg_abort_backup(int code, Datum arg)
}
}
+/*
+ * Create a consistent copy of control data to be used for backup and update it
+ * to require a backup label for recovery. Also recalculate the CRC.
+ */
+void
+backup_control_file(uint8_t *controlFile)
+{
+ ControlFileData *controlData = ((ControlFileData *)controlFile);
+
+ memset(controlFile, 0, PG_CONTROL_FILE_SIZE);
+
+ LWLockAcquire(ControlFileLock, LW_SHARED);
+ memcpy(controlFile, ControlFile, sizeof(ControlFileData));
+
+#ifdef USE_ASSERT_CHECKING
+ /*
+ * Verify that the contents of pg_control are the same in memory as on
disk
+ */
+ {
+ bool crc_ok;
+ ControlFileData *dataDisk = get_controlfile(DataDir, &crc_ok);
+
+ Assert(crc_ok &&
+ memcmp(dataDisk, controlFile,
sizeof(ControlFileData)) == 0);
+ }
+#endif
+
+ LWLockRelease(ControlFileLock);
+
+ controlData->backupLabelRequired = true;
+
+ INIT_CRC32C(controlData->crc);
+ COMP_CRC32C(controlData->crc, controlFile, offsetof(ControlFileData,
crc));
+ FIN_CRC32C(controlData->crc);
+}
+
/*
* Register a handler that will warn about unterminated backups at end of
* session, unless this has already been done.
diff --git a/src/backend/access/transam/xlogrecovery.c
b/src/backend/access/transam/xlogrecovery.c
index 6d2c4a86b96..bd702e895d7 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -647,7 +647,14 @@ InitWalRecovery(ControlFileData *ControlFile, bool
*wasShutdown_ptr,
}
else
{
- /* No backup_label file has been found if we are here. */
+ /*
+ * No backup_label file has been found if we are here. Error if
the
+ * control file requires backup_label.
+ */
+ if (ControlFile->backupLabelRequired)
+ ereport(FATAL,
+ (errmsg("could not find backup_label
required for recovery"),
+ errhint("backup_label must be present
for recovery to proceed")));
/*
* If tablespace_map file is present without backup_label file,
there
@@ -927,11 +934,21 @@ InitWalRecovery(ControlFileData *ControlFile, bool
*wasShutdown_ptr,
*
* Any other state indicates that the backup somehow became
corrupted
* and we can't sensibly continue with recovery.
+ *
+ * backupLabelRequired is set to false since backup_label is no
longer
+ * required once pg_control has been updated on disk. If
recovery
+ * terminates abnormally between when pg_control is updated and
+ * backup_label is renamed then on restart pg_control will be
+ * reinitialized from backup_label. If the user manually deletes
+ * backup_label before restarting then recovery will proceed
with the
+ * contents of pg_control just as it would if the crash had
happened
+ * directly after backup_label rename.
*/
if (haveBackupLabel)
{
ControlFile->backupStartPoint = checkPoint.redo;
ControlFile->backupEndRequired = backupEndRequired;
+ ControlFile->backupLabelRequired = false;
if (backupFromStandby)
{
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index ab1fbae8001..c4f7827f866 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -23,6 +23,7 @@
#include "backup/basebackup_incremental.h"
#include "backup/basebackup_sink.h"
#include "backup/basebackup_target.h"
+#include "catalog/pg_control.h"
#include "catalog/pg_tablespace_d.h"
#include "commands/defrem.h"
#include "common/compression.h"
@@ -332,9 +333,9 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
if (ti->path == NULL)
{
- struct stat statbuf;
bool sendtblspclinks = true;
char *backup_label;
+ uint8_t controlFile[PG_CONTROL_FILE_SIZE];
bbsink_begin_archive(sink, "base.tar");
@@ -357,14 +358,10 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
sendtblspclinks, &manifest,
InvalidOid, ib);
/* ... and pg_control after everything else. */
- if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0)
- ereport(ERROR,
-
(errcode_for_file_access(),
- errmsg("could not stat
file \"%s\": %m",
-
XLOG_CONTROL_FILE)));
- sendFile(sink, XLOG_CONTROL_FILE,
XLOG_CONTROL_FILE, &statbuf,
- false, InvalidOid, InvalidOid,
- InvalidRelFileNumber, 0,
&manifest, 0, NULL, 0);
+ backup_control_file(controlFile);
+ sendFileWithContent(sink, XLOG_CONTROL_FILE,
+ (char
*)controlFile, PG_CONTROL_FILE_SIZE,
+
&manifest);
}
else
{
diff --git a/src/backend/utils/misc/pg_controldata.c
b/src/backend/utils/misc/pg_controldata.c
index c6d9cbb1577..c2c19eb77df 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -162,8 +162,8 @@ pg_control_checkpoint(PG_FUNCTION_ARGS)
Datum
pg_control_recovery(PG_FUNCTION_ARGS)
{
- Datum values[5];
- bool nulls[5];
+ Datum values[6];
+ bool nulls[6];
TupleDesc tupdesc;
HeapTuple htup;
ControlFileData *ControlFile;
@@ -195,6 +195,9 @@ pg_control_recovery(PG_FUNCTION_ARGS)
values[4] = BoolGetDatum(ControlFile->backupEndRequired);
nulls[4] = false;
+ values[5] = BoolGetDatum(ControlFile->backupLabelRequired);
+ nulls[5] = false;
+
htup = heap_form_tuple(tupdesc, values, nulls);
PG_RETURN_DATUM(HeapTupleGetDatum(htup));
diff --git a/src/bin/pg_controldata/pg_controldata.c
b/src/bin/pg_controldata/pg_controldata.c
index a4060309ae0..5919dd58fed 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -301,6 +301,8 @@ main(int argc, char *argv[])
LSN_FORMAT_ARGS(ControlFile->backupEndPoint));
printf(_("End-of-backup record required: %s\n"),
ControlFile->backupEndRequired ? _("yes") : _("no"));
+ printf(_("Backup label required: %s\n"),
+ ControlFile->backupLabelRequired ? _("yes") : _("no"));
printf(_("wal_level setting: %s\n"),
wal_level_str(ControlFile->wal_level));
printf(_("wal_log_hints setting: %s\n"),
diff --git a/src/bin/pg_resetwal/pg_resetwal.c
b/src/bin/pg_resetwal/pg_resetwal.c
index ab766c34d4b..768a4ed2f18 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -918,6 +918,7 @@ RewriteControlFile(void)
ControlFile.backupStartPoint = InvalidXLogRecPtr;
ControlFile.backupEndPoint = InvalidXLogRecPtr;
ControlFile.backupEndRequired = false;
+ ControlFile.backupLabelRequired = false;
/*
* Force the defaults for max_* settings. The values don't really matter
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 9d745d4b25b..509b9e80a21 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -736,6 +736,7 @@ perform_rewind(filemap_t *filemap, rewind_source *source,
ControlFile_new.minRecoveryPoint = endrec;
ControlFile_new.minRecoveryPointTLI = endtli;
ControlFile_new.state = DB_IN_ARCHIVE_RECOVERY;
+ ControlFile_new.backupLabelRequired = true;
if (!dry_run)
update_controlfile(datadir_target, &ControlFile_new, do_sync);
}
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index dcc12eb8cbe..469739aa23f 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -313,6 +313,7 @@ extern void do_pg_backup_start(const char *backupidstr,
bool fast,
StringInfo
tblspcmapfile);
extern void do_pg_backup_stop(BackupState *state, bool waitforarchive);
extern void do_pg_abort_backup(int code, Datum arg);
+extern void backup_control_file(uint8_t *controlFile);
extern void register_persistent_abort_backup_handler(void);
extern SessionBackupState get_backup_status(void);
diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
index 77a661e818b..d9841f58108 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -166,12 +166,16 @@ typedef struct ControlFileData
* If backupEndRequired is true, we know for sure that we're restoring
* from a backup, and must see a backup-end record before we can safely
* start up.
+ *
+ * If backupLabelRequired is true, then a backup_label file must be
+ * present in order for recovery to proceed.
*/
XLogRecPtr minRecoveryPoint;
TimeLineID minRecoveryPointTLI;
XLogRecPtr backupStartPoint;
XLogRecPtr backupEndPoint;
bool backupEndRequired;
+ bool backupLabelRequired;
/*
* Parameter settings that determine if the WAL can be used for archival
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index fc8d82665b8..999ea2ac801 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12504,9 +12504,9 @@
{ oid => '3443',
descr => 'pg_controldata recovery state information as a function',
proname => 'pg_control_recovery', provolatile => 'v', prorettype => 'record',
- proargtypes => '', proallargtypes => '{pg_lsn,int4,pg_lsn,pg_lsn,bool}',
- proargmodes => '{o,o,o,o,o}',
- proargnames =>
'{min_recovery_end_lsn,min_recovery_end_timeline,backup_start_lsn,backup_end_lsn,end_of_backup_record_required}',
+ proargtypes => '', proallargtypes => '{pg_lsn,int4,pg_lsn,pg_lsn,bool,bool}',
+ proargmodes => '{o,o,o,o,o,o}',
+ proargnames =>
'{min_recovery_end_lsn,min_recovery_end_timeline,backup_start_lsn,backup_end_lsn,end_of_backup_record_required,backup_label_required}',
prosrc => 'pg_control_recovery' },
{ oid => '3444',
diff --git a/src/test/recovery/t/002_archiving.pl
b/src/test/recovery/t/002_archiving.pl
index aa40f58e6d6..9963d13473e 100644
--- a/src/test/recovery/t/002_archiving.pl
+++ b/src/test/recovery/t/002_archiving.pl
@@ -41,6 +41,26 @@ $node_standby->append_conf(
archive_cleanup_command = 'echo archive_cleanup_done >
$archive_cleanup_command_file'
recovery_end_command = 'echo recovery_ended_done > $recovery_end_command_file'
));
+
+# Rename backup_label to verify that recovery will not start without it
+rename("$data_dir/backup_label", "$data_dir/backup_label.tmp")
+ or BAIL_OUT "could not move $data_dir/backup_label";
+
+my $res = run_log(
+ [
+ 'pg_ctl', '-D', $node_standby->data_dir, '-l',
+ $node_standby->logfile, 'start'
+ ]);
+ok(!$res, 'invalid recovery startup fails');
+
+my $logfile = slurp_file($node_standby->logfile());
+ok($logfile =~ qr/could not find backup_label required for recovery/,
+ 'could not find backup_label required for recovery');
+
+# Restore backup_label so recovery proceeds normally
+rename("$data_dir/backup_label.tmp", "$data_dir/backup_label")
+ or BAIL_OUT "could not move $data_dir/backup_label";
+
$node_standby->start;
# Create some content on primary
--
2.34.1
From 5e19f24aa151e7502799e26c5d9ac7d574b4750f Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Wed, 18 Mar 2026 07:11:13 +0000
Subject: Return pg_control from pg_backup_stop().
Harden recovery by returning a copy of pg_control from pg_backup_stop() that has
a flag set to prevent recovery if the backup_label file is missing. Instead of
backup software copying pg_control from PGDATA, it stores an updated version
that is returned from pg_backup_stop(). This is better for the following
reasons:
* The user can no longer remove backup_label and get what looks like a
successful recovery (while almost certainly causing corruption). If backup_label
is removed the cluster will not start. The user may try pg_resetwal, but that
tool makes it pretty clear that corruption will result from its use.
* We don't need to worry about backup software seeing a torn copy of pg_control,
since Postgres can safely read it out of memory and provide a valid copy via
pg_backup_stop(). This solves torn reads without needing to write pg_control via
a temp file, which may affect performance on a standby.
* For backup from standby, we no longer need to instruct the backup software to
copy pg_control last. In fact the backup software should not copy pg_control
from
PGDATA at all.
These changes have no impact on current backup software and they are free to use
the pg_control available from pg_stop_backup() or continue to use pg_control
from
PGDATA. Of course they will miss the benefits of getting a consistent copy of
pg_control and the backup_label checking, but will be no worse off than before.
Catalog version bump is required.
---
doc/src/sgml/backup.sgml | 18 +++++-
src/backend/access/transam/xlogfuncs.c | 20 ++++--
src/include/catalog/pg_proc.dat | 4 +-
src/test/recovery/t/042_low_level_backup.pl | 67 ++++++++++++++++++++-
4 files changed, 97 insertions(+), 12 deletions(-)
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 168444eccc5..67974b7b1b6 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1027,9 +1027,12 @@ SELECT * FROM pg_backup_stop(wait_for_archive => true);
values. The second of these fields should be written to a file named
<filename>backup_label</filename> in the root directory of the backup. The
third field should be written to a file named
- <filename>tablespace_map</filename> unless the field is empty. These
files are
+ <filename>tablespace_map</filename> unless the field is empty. The fourth
+ field should be written into a file named
+ <filename>global/pg_control</filename> (overwriting the existing file when
+ present). These files are
vital to the backup working and must be written byte for byte without
- modification, which may require opening the file in binary mode.
+ modification, which will require opening the file in binary mode.
</para>
</listitem>
<listitem>
@@ -1101,7 +1104,16 @@ SELECT * FROM pg_backup_stop(wait_for_archive => true);
</para>
<para>
- You should, however, omit from the backup the files within the
+ You should exclude <filename>global/pg_control</filename> from your backup
+ and put the contents of the <parameter>controlfile</parameter> column
+ returned from <function>pg_backup_stop</function> in your backup at
+ <filename>global/pg_control</filename>. This version of pg_control contains
+ safeguards against recovery without backup_label present and is guaranteed
+ not to be torn.
+ </para>
+
+ <para>
+ You should also omit from the backup the files within the
cluster's <filename>pg_wal/</filename> subdirectory. This
slight adjustment is worthwhile because it reduces the risk
of mistakes when restoring. This is easy to arrange if
diff --git a/src/backend/access/transam/xlogfuncs.c
b/src/backend/access/transam/xlogfuncs.c
index 4e35311b2f3..fab0c047621 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -143,9 +143,11 @@ pg_backup_start(PG_FUNCTION_ARGS)
*
* The backup_label contains the user-supplied label string (typically this
* would be used to tell where the backup dump will be stored), the starting
- * time, starting WAL location for the dump and so on. It is the caller's
- * responsibility to write the backup_label and tablespace_map files in the
- * data folder that will be restored from this backup.
+ * time, starting WAL location for the dump and so on. The pg_control file
+ * contains a consistent copy of pg_control that also has a safeguard against
+ * being used without backup_label. It is the caller's responsibility to write
+ * the backup_label, pg_control, and tablespace_map files in the data folder
+ * that will be restored from this backup.
*
* Permission checking for this function is managed through the normal
* GRANT system.
@@ -153,12 +155,14 @@ pg_backup_start(PG_FUNCTION_ARGS)
Datum
pg_backup_stop(PG_FUNCTION_ARGS)
{
-#define PG_BACKUP_STOP_V2_COLS 3
+#define PG_BACKUP_STOP_V2_COLS 4
TupleDesc tupdesc;
Datum values[PG_BACKUP_STOP_V2_COLS] = {0};
bool nulls[PG_BACKUP_STOP_V2_COLS] = {0};
bool waitforarchive = PG_GETARG_BOOL(0);
char *backup_label;
+ bytea *pg_control_bytea;
+ uint8_t pg_control[PG_CONTROL_FILE_SIZE];
SessionBackupState status = get_backup_status();
/* Initialize attributes information in the tuple descriptor */
@@ -174,6 +178,13 @@ pg_backup_stop(PG_FUNCTION_ARGS)
Assert(backup_state != NULL);
Assert(tablespace_map != NULL);
+ /* Build the contents of pg_control */
+ backup_control_file(pg_control);
+
+ pg_control_bytea = (bytea *) palloc(PG_CONTROL_FILE_SIZE + VARHDRSZ);
+ SET_VARSIZE(pg_control_bytea, PG_CONTROL_FILE_SIZE + VARHDRSZ);
+ memcpy(VARDATA(pg_control_bytea), pg_control, PG_CONTROL_FILE_SIZE);
+
/* Stop the backup */
do_pg_backup_stop(backup_state, waitforarchive);
@@ -183,6 +194,7 @@ pg_backup_stop(PG_FUNCTION_ARGS)
values[0] = LSNGetDatum(backup_state->stoppoint);
values[1] = CStringGetTextDatum(backup_label);
values[2] = CStringGetTextDatum(tablespace_map->data);
+ values[3] = PointerGetDatum(pg_control_bytea);
/* Deallocate backup-related variables */
pfree(backup_label);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 999ea2ac801..ce717e8828c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6767,8 +6767,8 @@
{ oid => '2739', descr => 'finish taking an online backup',
proname => 'pg_backup_stop', provolatile => 'v', proparallel => 'r',
prorettype => 'record', proargtypes => 'bool',
- proallargtypes => '{bool,pg_lsn,text,text}', proargmodes => '{i,o,o,o}',
- proargnames => '{wait_for_archive,lsn,labelfile,spcmapfile}',
+ proallargtypes => '{bool,pg_lsn,text,text,bytea}', proargmodes =>
'{i,o,o,o,o}',
+ proargnames => '{wait_for_archive,lsn,labelfile,spcmapfile,controlfile}',
proargdefaults => '{true}',
prosrc => 'pg_backup_stop',
proacl => '{POSTGRES=X}' },
diff --git a/src/test/recovery/t/042_low_level_backup.pl
b/src/test/recovery/t/042_low_level_backup.pl
index df4ae029fe6..34c6f4461af 100644
--- a/src/test/recovery/t/042_low_level_backup.pl
+++ b/src/test/recovery/t/042_low_level_backup.pl
@@ -13,6 +13,42 @@ use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
+# Decode hex to binary
+sub decode_hex
+{
+ my ($encoded) = @_;
+ my $decoded;
+
+ $encoded =~ s/^\s+|\s+$//g;
+
+ for (my $idx = 0; $idx < length($encoded); $idx += 2)
+ {
+ $decoded .= pack('C', hex(substr($encoded, $idx, 2)));
+ }
+
+ return $decoded;
+}
+
+# Get backup_label/pg_control from pg_stop_backup()
+sub stop_backup_result
+{
+ my ($psql) = @_;
+
+ my $encoded = $psql->query_safe(
+ "select encode(labelfile::bytea, 'hex') || ',' || " .
+ " encode(controlfile, 'hex')" .
+ " from pg_backup_stop()");
+
+ my @result;
+
+ foreach my $column (split(',', $encoded))
+ {
+ push(@result, decode_hex($column));
+ }
+
+ return @result;
+}
+
# Start primary node with archiving.
my $node_primary = PostgreSQL::Test::Cluster->new('primary');
$node_primary->init(has_archiving => 1, allows_streaming => 1);
@@ -80,8 +116,7 @@ my $stop_segment_name = $node_primary->safe_psql('postgres',
'SELECT pg_walfile_name(pg_current_wal_lsn())');
# Stop backup and get backup_label, the last segment is archived.
-my $backup_label =
- $psql->query_safe("select labelfile from pg_backup_stop()");
+(my $backup_label, my $pg_control) = stop_backup_result($psql);
$psql->quit;
@@ -118,10 +153,36 @@ ok( $node_replica->log_contains(
$node_replica->teardown_node;
$node_replica->clean_node;
+# Save only pg_control into the backup to demonstrate that pg_control returned
+# from pg_stop_backup() will only perform recovery when backup_label is
present.
+open(my $fh, ">", "$backup_dir/global/pg_control")
+ or die "could not open pg_control";
+binmode($fh);
+syswrite($fh, $pg_control);
+close($fh);
+
+$node_replica = PostgreSQL::Test::Cluster->new('replica_fail2');
+$node_replica->init_from_backup($node_primary, $backup_name,
+ has_restoring => 1);
+
+my $res = run_log(
+ [
+ 'pg_ctl', '-D', $node_replica->data_dir, '-l',
+ $node_replica->logfile, 'start'
+ ]);
+ok(!$res, 'invalid recovery startup fails');
+
+my $logfile = slurp_file($node_replica->logfile());
+ok($logfile =~ qr/could not find backup_label required for recovery/,
+ 'could not find backup_label required for recovery');
+
+$node_replica->teardown_node;
+$node_replica->clean_node;
+
# Save backup_label into the backup directory and recover using the primary's
# archive. This time recovery will succeed and the canary table will be
# present.
-open my $fh, ">>", "$backup_dir/backup_label"
+open $fh, ">>", "$backup_dir/backup_label"
or die "could not open backup_label";
# Binary mode is required for Windows, as the backup_label parsing is not
# able to cope with CRLFs.
--
2.34.1
