Hi Shihao,
On 8/28/26 05:34, shihao zhong wrote:
Thanks for working on this -- it fills a gap that I think Postgres has
needed for a long time.
I reviewed and tested v11 on master and it looks all good.
Thank you for having a look at the patch.
One thing I think needs a decision, which I left alone. xlogrecovery.c
still suggests removing backup_label in two places. Restore a
"pg_basebackup -X none" backup without recovery.signal and you get
"could not locate required checkpoint record", hinting "try removing the
file backup_label". Do that, and you get "could not find backup_label
required for recovery" telling you to put it back.
Agreed, and I have addressed it in v13 by adding a conditional to the
relevant errors with a alternate message:
ControlFile->backupLabelRequired ?
errhint("Touch \"%s/recovery.signal\" or \"%s/standby.signal\" and add
required recovery options.\n"
"Do not remove \"%s/backup_label\"; it is required to recover this
backup.",
DataDir, DataDir, DataDir) :
Smaller things:
1. errhint() should start with a capital letter ("Restore the ...").
Fixed in v12, along with "(char *)controlFile" -> "(char *) controlFile".
pg_indent would catch this, but always better to have to source as clean
as possible. Added into v13.
2. The commit message says "limited to pg_basebackup", but the patch also
sets the flag in pg_rewind. The logic is fine; v12 just says so, and
it probably deserves a line in pg_rewind.sgml.
Changes to the commit message preserved in v13. Docs updated per below.
3. Assert(crc_ok && memcmp(...) == 0) combines two conditions; splitting
them would say which one failed.
I left this as is from v11. I think it's a pretty normal thing to do. If
you are running with asserts then you are debugging anyway so resolving
the missed condition is pretty trivial, IMO.
4. v12 moves the test out of 002_archiving.pl, which is about
archive_cleanup_command, into 057_backup_label_required.pl. Your
assertions are kept, plus checks that the flag is set in the backup,
cleared after recovery, and the same for a backup from a standby.
Yes, I think a dedicated test module makes sense here, so I kept that in
v13.
5. Docs are untouched beyond the pg_control_recovery() table. At least
pg_basebackup.sgml needs a mention, and something in backup.sgml about
low-level backups not getting this protection now that the SQL part is
dropped. Worth saying that pg_resetwal is the only way to clear the
flag, too.
Docs have been updated with each of these suggestions.
I also added a few additional tests to flesh out the coverage.
Regards,
-David
From 53c087c4daadc1d88fcad68bc3045ba01b9a6c6e Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Tue, 15 Sep 2026 12:48:33 +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.
Recovery hints that suggest removing backup_label are omitted when the flag is
set, since in that case pg_control shows that the data directory came from a
backup and removing backup_label would leave it unable to start at all.
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 and pg_rewind (or any software
comfortable with modifying pg_control). pg_rewind creates a backup_label of its
own, so the flag is set there as well, which means that removing backup_label
after a rewind no longer allows the cluster to start.
Control and catalog version bumps are required.
---
doc/src/sgml/backup.sgml | 13 ++
doc/src/sgml/func/func-info.sgml | 5 +
doc/src/sgml/ref/pg_basebackup.sgml | 10 ++
doc/src/sgml/ref/pg_rewind.sgml | 4 +
src/backend/access/transam/xlog.c | 47 +++++
src/backend/access/transam/xlogrecovery.c | 27 ++-
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/bin/pg_rewind/t/RewindTest.pm | 12 ++
src/include/access/xlog.h | 1 +
src/include/catalog/pg_control.h | 4 +
src/include/catalog/pg_proc.dat | 6 +-
src/test/recovery/meson.build | 1 +
src/test/recovery/t/042_low_level_backup.pl | 8 +
.../recovery/t/057_backup_label_required.pl | 165 ++++++++++++++++++
18 files changed, 314 insertions(+), 15 deletions(-)
create mode 100644 src/test/recovery/t/057_backup_label_required.pl
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 168444eccc5..984984b8959 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1031,6 +1031,19 @@ SELECT * FROM pg_backup_stop(wait_for_archive => true);
vital to the backup working and must be written byte for byte without
modification, which may require opening the file in binary mode.
</para>
+ <para>
+ Note that a backup made with the low-level API copies
+ <filename>pg_control</filename> from the running cluster, so it does not
+ contain the flag that <xref linkend="app-pgbasebackup"/> sets to make
+ <filename>backup_label</filename> mandatory for recovery. Removing
+ <filename>backup_label</filename> from such a backup will not prevent the
+ cluster from starting; it will instead recover from the checkpoint
+ recorded in <filename>pg_control</filename>. Unless that happens to be the
+ checkpoint the backup started from, recovery will begin too late and
+ silently produce an inconsistent cluster.
+ Backup software using this API should set the flag itself if it is able to
+ modify <filename>pg_control</filename>.
+ </para>
</listitem>
<listitem>
<para>
diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml
index e56c9a22c42..f7ce58479a9 100644
--- a/doc/src/sgml/func/func-info.sgml
+++ b/doc/src/sgml/func/func-info.sgml
@@ -3644,6 +3644,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/doc/src/sgml/ref/pg_basebackup.sgml
b/doc/src/sgml/ref/pg_basebackup.sgml
index 3117968d125..9bc6210fde2 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -60,6 +60,16 @@ PostgreSQL documentation
<xref linkend="app-pgdump"/> must be used.
</para>
+ <para>
+ The copy of <filename>pg_control</filename> stored in the backup is flagged
+ to require the <filename>backup_label</filename> file for recovery. If
+ <filename>backup_label</filename> is removed, recovery refuses to start
+ rather than silently producing an inconsistent cluster. The flag is cleared
+ automatically once recovery has read <filename>backup_label</filename>;
+ <xref linkend="app-pgresetwal"/> is the only way to clear it otherwise, and
+ doing so on a backup that has not been recovered will corrupt it.
+ </para>
+
<para>
The backup is made over a regular <productname>PostgreSQL</productname>
connection that uses the replication protocol. The connection must be made
diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index ac3d0c9328f..c4888c183fb 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -455,6 +455,10 @@ GRANT EXECUTE ON FUNCTION
pg_catalog.pg_read_binary_file(text, bigint, bigint, b
defined as the result of <literal>pg_current_wal_insert_lsn()</literal>
when rewinding from a live source or the last checkpoint LSN when
rewinding from a stopped source.
+ <filename>pg_control</filename> is also flagged to require
+ <filename>backup_label</filename>, so that removing that file does not
+ allow the target to start up in an inconsistent state. The flag is
+ cleared once the target has processed <filename>backup_label</filename>.
</para>
</step>
<step>
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 9ec0be77ca0..f44651d5eb5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10589,6 +10589,53 @@ 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.
+ *
+ * The caller's buffer must be at least PG_CONTROL_FILE_SIZE bytes and is
+ * zero-padded to that size.
+ *
+ * All field access is done through a local, properly-aligned ControlFileData;
+ * the caller's buffer is only ever written via memcpy() and so need not be
+ * suitably aligned for ControlFileData.
+ */
+void
+backup_control_file(uint8 *controlFile)
+{
+ ControlFileData controlData;
+
+ LWLockAcquire(ControlFileLock, LW_SHARED);
+ memcpy(&controlData, 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, &controlData,
sizeof(ControlFileData)) == 0);
+
+ pfree(dataDisk);
+ }
+#endif
+
+ LWLockRelease(ControlFileLock);
+
+ controlData.backupLabelRequired = true;
+
+ INIT_CRC32C(controlData.crc);
+ COMP_CRC32C(controlData.crc, &controlData, offsetof(ControlFileData,
crc));
+ FIN_CRC32C(controlData.crc);
+
+ /* Copy into the caller's buffer, zero-padded to the full file size */
+ memset(controlFile, 0, PG_CONTROL_FILE_SIZE);
+ memcpy(controlFile, &controlData, sizeof(ControlFileData));
+}
+
/*
* 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 fff8d57ac61..f61e4c0199b 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -604,6 +604,10 @@ InitWalRecovery(ControlFileData *ControlFile, bool
*wasShutdown_ptr,
ereport(FATAL,
errmsg("could not find
redo location %X/%08X referenced by checkpoint record at %X/%08X",
LSN_FORMAT_ARGS(checkPoint.redo), LSN_FORMAT_ARGS(CheckPointLoc)),
+
ControlFile->backupLabelRequired ?
+ errhint("Touch
\"%s/recovery.signal\" or \"%s/standby.signal\" and add required recovery
options.\n"
+ "Do not
remove \"%s/backup_label\"; it is required to recover this backup.",
+
DataDir, DataDir, DataDir) :
errhint("If you are
restoring from a backup, touch \"%s/recovery.signal\" or \"%s/standby.signal\"
and add required recovery options.\n"
"If you
are not restoring from a backup, try removing the file \"%s/backup_label\".\n"
"Be
careful: removing \"%s/backup_label\" will result in a corrupt cluster if
restoring from a backup.",
@@ -615,6 +619,10 @@ InitWalRecovery(ControlFileData *ControlFile, bool
*wasShutdown_ptr,
ereport(FATAL,
errmsg("could not locate required
checkpoint record at %X/%08X",
LSN_FORMAT_ARGS(CheckPointLoc)),
+ ControlFile->backupLabelRequired ?
+ errhint("Touch \"%s/recovery.signal\"
or \"%s/standby.signal\" and add required recovery options.\n"
+ "Do not remove
\"%s/backup_label\"; it is required to recover this backup.",
+ DataDir, DataDir,
DataDir) :
errhint("If you are restoring from a
backup, touch \"%s/recovery.signal\" or \"%s/standby.signal\" and add required
recovery options.\n"
"If you are not
restoring from a backup, try removing the file \"%s/backup_label\".\n"
"Be careful: removing
\"%s/backup_label\" will result in a corrupt cluster if restoring from a
backup.",
@@ -659,7 +667,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("Restore the backup_label file
that was created during the backup."));
/*
* If tablespace_map file is present without backup_label file,
there
@@ -939,11 +954,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 e3c04ecd810..74ab7781512 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"
@@ -340,9 +341,9 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
if (ti->path == NULL)
{
- struct stat statbuf;
bool sendtblspclinks = true;
char *backup_label;
+ uint8
controlFile[PG_CONTROL_FILE_SIZE];
bbsink_begin_archive(sink, "base.tar");
@@ -365,14 +366,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 1f47302fe2a..232c888d594 100644
--- a/src/backend/utils/misc/pg_controldata.c
+++ b/src/backend/utils/misc/pg_controldata.c
@@ -168,8 +168,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;
@@ -201,6 +201,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 6a0f848d8d0..8bcca89733b 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -305,6 +305,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 41afc4c1316..9d36c5b4a84 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -922,6 +922,7 @@ RewriteControlFile(void)
ControlFile.backupStartPoint = InvalidXLogRecPtr;
ControlFile.backupEndPoint = InvalidXLogRecPtr;
ControlFile.backupEndRequired = false;
+ ControlFile.backupLabelRequired = false;
/*
* The old WAL is gone and the new position may lie below the old
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index d2521dab333..a16081669e1 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -783,6 +783,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;
/*
* Keep the target's own data checksum state. Most of the data
directory
diff --git a/src/bin/pg_rewind/t/RewindTest.pm
b/src/bin/pg_rewind/t/RewindTest.pm
index 32aeca80f13..b2a7fca9d02 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -353,6 +353,18 @@ sub run_pg_rewind
croak("Incorrect test mode specified");
}
+ # pg_rewind writes a backup_label of its own when it rewinds, and the
+ # target must then require it for recovery; the flag is cleared again
when
+ # the target is restarted below and recovery consumes backup_label.
When
+ # the target is already an ancestor of the source no rewind takes place,
+ # and pg_control must be left alone so that the target still starts.
+ my $label_required =
+ -f "$primary_pgdata/backup_label" ? 'yes' : 'no';
+ command_like(
+ [ 'pg_controldata', '--pgdata' => $primary_pgdata ],
+ qr/Backup label required: +$label_required/,
+ 'backup_label requirement matches whether a rewind took place');
+
# Now move back postgresql.conf with old settings
move(
"$tmp_folder/primary-postgresql.conf.tmp",
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 7a590b7e1ea..23f42bc7602 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -340,6 +340,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 *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 c3c934d0012..f83928e2ca3 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -172,12 +172,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 f46427258e3..19523bc882e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12442,9 +12442,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/meson.build b/src/test/recovery/meson.build
index 72113c5ac6e..082dfb17c00 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -65,6 +65,7 @@ tests += {
't/054_unlogged_sequence_promotion.pl',
't/055_cascade_reconnect.pl',
't/056_standby_snapshot_export.pl',
+ 't/057_backup_label_required.pl',
],
},
}
diff --git a/src/test/recovery/t/042_low_level_backup.pl
b/src/test/recovery/t/042_low_level_backup.pl
index 7ed54e611eb..0e2cc7ee8ac 100644
--- a/src/test/recovery/t/042_low_level_backup.pl
+++ b/src/test/recovery/t/042_low_level_backup.pl
@@ -165,4 +165,12 @@ ok( $node_replica->log_contains(
'FATAL: .*could not locate required checkpoint record at'),
'ends with FATAL for missing required checkpoint record');
+# A low-level backup copies pg_control from the running cluster, so it does not
+# require backup_label and the hint still offers removing the file as a way to
+# recover a cluster that is not being restored from a backup. Backups made
+# with pg_basebackup do require it and get a different hint; see
+# 057_backup_label_required.pl.
+ok($node_replica->log_contains('try removing the file .*backup_label'),
+ 'hint offers removing backup_label when it is not required');
+
done_testing();
diff --git a/src/test/recovery/t/057_backup_label_required.pl
b/src/test/recovery/t/057_backup_label_required.pl
new file mode 100644
index 00000000000..cdf927af596
--- /dev/null
+++ b/src/test/recovery/t/057_backup_label_required.pl
@@ -0,0 +1,165 @@
+# Copyright (c) 2021-2026, PostgreSQL Global Development Group
+
+# Test the pg_control flag that makes backup_label mandatory for recovery.
+#
+# pg_basebackup stores a modified copy of pg_control in the backup, with a flag
+# set that makes recovery refuse to start if backup_label is missing. This
+# prevents the silent corruption that results from removing the file, both for
+# backups taken from a primary and from a standby.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::RecursiveCopy;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Rename backup_label in the data directory of the given node, start it, and
+# check that recovery refuses to proceed. Then put the file back.
+sub check_startup_without_backup_label
+{
+ my ($node, $test_name) = @_;
+ my $data_dir = $node->data_dir;
+ my $log_offset = -s $node->logfile;
+
+ rename("$data_dir/backup_label", "$data_dir/backup_label.tmp")
+ or BAIL_OUT("could not rename $data_dir/backup_label");
+
+ is($node->start(fail_ok => 1), 0, "$test_name: startup fails");
+ ok( $node->log_contains(
+ 'FATAL: .*could not find backup_label required for
recovery',
+ $log_offset),
+ "$test_name: ends with FATAL for missing backup_label");
+
+ rename("$data_dir/backup_label.tmp", "$data_dir/backup_label")
+ or BAIL_OUT("could not rename $data_dir/backup_label.tmp");
+ return;
+}
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+
+# Set wal_keep_size to prevent WAL segment recycling after the checkpoints
+# enforced by the backups below. The standby is created from the first backup
+# but started only after later backups have advanced the primary, so it must
+# still be able to stream the segments in between. This is set before the
+# first backup so that it is inherited by the standbys.
+$node_primary->append_conf('postgresql.conf', 'wal_keep_size = 64MB');
+$node_primary->start;
+
+$node_primary->safe_psql('postgres',
+ 'CREATE TABLE tab_int AS SELECT generate_series(1, 1000) AS a');
+
+# Take a backup from the primary. The copy of pg_control stored in the backup
+# must require backup_label, while the control file of the running cluster is
+# left alone.
+my $backup_name = 'backup_primary';
+$node_primary->backup($backup_name);
+
+command_like(
+ [
+ 'pg_controldata',
+ '--pgdata' => $node_primary->backup_dir . '/' . $backup_name
+ ],
+ qr/Backup label required: +yes/,
+ 'backup taken from a primary requires backup_label');
+command_like(
+ [ 'pg_controldata', '--pgdata' => $node_primary->data_dir ],
+ qr/Backup label required: +no/,
+ 'control file of the source cluster is unchanged');
+is( $node_primary->safe_psql(
+ 'postgres', 'SELECT backup_label_required FROM
pg_control_recovery()'),
+ 'f',
+ 'pg_control_recovery() reports the flag not set on the source cluster');
+
+# pg_resetwal is the only supported way to clear the flag without recovering.
+# Use a copy so the original backup is left intact for the restore tests below.
+my $reset_dir = $node_primary->backup_dir . '/' . $backup_name . '_reset';
+PostgreSQL::Test::RecursiveCopy::copypath(
+ $node_primary->backup_dir . '/' . $backup_name, $reset_dir);
+chmod(0700, $reset_dir) or BAIL_OUT("could not chmod $reset_dir");
+
+command_ok([ 'pg_resetwal', '--force', '--pgdata' => $reset_dir ],
+ 'pg_resetwal runs on a backup that requires backup_label');
+command_like(
+ [ 'pg_controldata', '--pgdata' => $reset_dir ],
+ qr/Backup label required: +no/,
+ 'pg_resetwal clears the flag');
+
+# Restoring that backup without backup_label must not start.
+my $node_restored = PostgreSQL::Test::Cluster->new('restored');
+$node_restored->init_from_backup($node_primary, $backup_name);
+
+check_startup_without_backup_label($node_restored, 'backup from primary');
+
+# With backup_label back in place recovery completes, and the flag is cleared
+# so that subsequent restarts no longer need the file.
+$node_restored->start;
+is($node_restored->safe_psql('postgres', 'SELECT count(*) FROM tab_int'),
+ 1000, 'restored cluster has the expected contents');
+is( $node_restored->safe_psql(
+ 'postgres', 'SELECT backup_label_required FROM
pg_control_recovery()'),
+ 'f',
+ 'flag is cleared once recovery has completed');
+$node_restored->stop;
+
+command_like(
+ [ 'pg_controldata', '--pgdata' => $node_restored->data_dir ],
+ qr/Backup label required: +no/,
+ 'control file no longer requires backup_label after recovery');
+
+# A backup made without WAL cannot reach a consistent state on its own, and
+# recovery reports that it could not locate the checkpoint record. Since
+# pg_control requires backup_label, the hint for that error must not repeat the
+# usual advice to remove the file.
+my $nowal_backup = 'backup_nowal';
+$node_primary->backup($nowal_backup,
+ backup_options => [ '--wal-method' => 'none' ]);
+
+my $node_nowal = PostgreSQL::Test::Cluster->new('nowal');
+$node_nowal->init_from_backup($node_primary, $nowal_backup);
+
+my $nowal_offset = -s $node_nowal->logfile;
+is($node_nowal->start(fail_ok => 1), 0,
+ 'backup without WAL fails to start when no recovery options are set');
+ok( $node_nowal->log_contains(
+ 'FATAL: .*could not locate required checkpoint record',
$nowal_offset),
+ 'missing checkpoint record is reported');
+ok( $node_nowal->log_contains('HINT: .*recovery\.signal', $nowal_offset),
+ 'hint points at the recovery signal files');
+ok( $node_nowal->log_contains('Do not remove .*backup_label', $nowal_offset),
+ 'hint tells the user to keep backup_label');
+ok( !$node_nowal->log_contains('try removing the file', $nowal_offset),
+ 'hint does not suggest removing backup_label');
+
+# A backup taken from a standby gets the same treatment. This is the case that
+# previously required backup software to copy pg_control last.
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+$node_primary->wait_for_replay_catchup($node_standby);
+
+my $standby_backup = 'backup_standby';
+$node_standby->backup($standby_backup);
+
+command_like(
+ [
+ 'pg_controldata',
+ '--pgdata' => $node_standby->backup_dir . '/' . $standby_backup
+ ],
+ qr/Backup label required: +yes/,
+ 'backup taken from a standby requires backup_label');
+
+my $node_standby2 = PostgreSQL::Test::Cluster->new('standby2');
+$node_standby2->init_from_backup($node_standby, $standby_backup,
+ has_streaming => 1);
+
+check_startup_without_backup_label($node_standby2, 'backup from standby');
+
+$node_standby2->start;
+$node_standby->wait_for_replay_catchup($node_standby2, $node_primary);
+is($node_standby2->safe_psql('postgres', 'SELECT count(*) FROM tab_int'),
+ 1000, 'cascading standby from a standby backup is caught up');
+
+done_testing();
--
2.34.1
