On 10/21/26 15:07, David Steele wrote:
Patch has been rebased.
A committer pointed out to me that part of this patch fixes a bug and
could be back-patched, so it should be separate.
Patch 01 now fixes only the possibility of a torn pg_control file. It
makes the copy out of memory to reduce the time the lock is held, but it
could alternately read the pg_control file in the lock as was done by
Thomas in c558e6fd92f. If the consensus is that the latter approach is
preferred I'm happy to make that change.
Patch 02 implements the flag to enforce the presence of backup_label and
credits Andres with the original idea. This is the bulk of the prior patch.
Regards,
-David
From 8a2822f52faa46955be97340741310327351374f Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Fri, 23 Oct 2026 12:33:19 +0000
Subject: Avoid a torn copy of pg_control in base backups.
Base backups read pg_control from disk, which can return a torn copy if the
server is rewriting the file at the same time. At least ext4 and ntfs can
expose partially written contents to a concurrent read, and a backup
containing a torn pg_control fails to start because the CRC does not match.
Commit 63a582222c6 dealt with the same hazard in frontend tools by retrying
the read until the CRC is stable, which is appropriate for programs that have
not access to shared memory. Commit c558e6fd92f made the SQL functions take
ControlFileLock while reading the file. A base backup runs in the server,
which already has the contents of pg_control in shared memory, so instead copy
them from there under ControlFileLock. No read of the file is needed at all,
and the lock is held only for the copy.
The CRC is recalculated because the in-memory copy can be updated without
being written, e.g. during replay of an online checkpoint on a standby, so
the CRC it holds cannot be relied on.
Backups taken with the low-level API still copy pg_control from disk and are
not helped by this change.
---
src/backend/access/transam/xlog.c | 36 +++++++++++++++++++++++++++++++
src/backend/backup/basebackup.c | 15 ++++++-------
src/include/access/xlog.h | 1 +
3 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 8636ad9ab2c..e89227ed8b9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10608,6 +10608,42 @@ do_pg_abort_backup(int code, Datum arg)
}
}
+/*
+ * Create a consistent copy of control data to be used for backup.
+ *
+ * Reading pg_control from disk while it is being rewritten can return a torn
+ * copy, so make the copy from shared memory under ControlFileLock instead.
+ *
+ * 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));
+ LWLockRelease(ControlFileLock);
+
+ /*
+ * Recalculate the CRC. The in-memory copy can be updated without being
+ * written, e.g. during replay of an online checkpoint, in which case
its
+ * CRC does not match its contents.
+ */
+ 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/backup/basebackup.c b/src/backend/backup/basebackup.c
index d7c485dc002..c9ee9e2cac8 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/include/access/xlog.h b/src/include/access/xlog.h
index 2bf5714b51e..a2a640178c4 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -343,6 +343,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);
--
2.34.1
From 54689b92d326f7e25897458d8b4b45562eeb7f66 Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Fri, 23 Oct 2026 12:33:19 +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.
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.
Andres Freund suggested this approach in an earlier thread that proposed
moving the contents of backup_label into pg_control.
Control and catalog version bumps are required.
Suggested-by: Andres Freund <[email protected]>
Discussion:
https://postgr.es/m/20231120203746.x3dgftmwvfgl5tdy%40awork3.anarazel.de
---
doc/src/sgml/backup.sgml | 13 ++
doc/src/sgml/func/func-info.sgml | 5 +
doc/src/sgml/ref/pg_basebackup.sgml | 12 ++
doc/src/sgml/ref/pg_rewind.sgml | 4 +
src/backend/access/transam/xlog.c | 5 +-
src/backend/access/transam/xlogrecovery.c | 27 ++-
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/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/061_backup_label_required.pl | 165 ++++++++++++++++++
16 files changed, 266 insertions(+), 7 deletions(-)
create mode 100644 src/test/recovery/t/061_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 c9c0261b52f..2f0651c694e 100644
--- a/doc/src/sgml/func/func-info.sgml
+++ b/doc/src/sgml/func/func-info.sgml
@@ -3651,6 +3651,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 b4a0faccf41..93676057394 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -60,6 +60,18 @@ 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. Restoring
+ <filename>backup_label</filename> to the root of the backup allows
+ recovery to proceed, and the flag is cleared once recovery has read the
+ file. <xref linkend="app-pgresetwal"/> also clears the flag, but discarding
+ the WAL bypasses the replay that recovery needs, leaving an inconsistent
+ cluster.
+ </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 e89227ed8b9..3ae2deb787e 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10609,7 +10609,8 @@ do_pg_abort_backup(int code, Datum arg)
}
/*
- * Create a consistent copy of control data to be used for backup.
+ * Create a consistent copy of control data to be used for backup and flag it
+ * to require backup_label for recovery.
*
* Reading pg_control from disk while it is being rewritten can return a torn
* copy, so make the copy from shared memory under ControlFileLock instead.
@@ -10630,6 +10631,8 @@ backup_control_file(uint8 *controlFile)
memcpy(&controlData, ControlFile, sizeof(ControlFileData));
LWLockRelease(ControlFileLock);
+ controlData.backupLabelRequired = true;
+
/*
* Recalculate the CRC. The in-memory copy can be updated without being
* written, e.g. during replay of an online checkpoint, in which case
its
diff --git a/src/backend/access/transam/xlogrecovery.c
b/src/backend/access/transam/xlogrecovery.c
index 20be788da78..56368a9378f 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/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 634d966da9e..1b9846fb8a4 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -924,6 +924,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/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 1f21a8eb6d4..a885c173c25 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12443,9 +12443,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 b6ac9dd07a7..31b0cbd34e1 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -69,6 +69,7 @@ tests += {
't/058_shutdown_crash_restart.pl',
't/059_remote_apply_status_interval.pl',
't/060_multixact_crashed_updater.pl',
+ 't/061_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..8633b690cc6 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
+# 061_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/061_backup_label_required.pl
b/src/test/recovery/t/061_backup_label_required.pl
new file mode 100644
index 00000000000..5d1cee27054
--- /dev/null
+++ b/src/test/recovery/t/061_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 clears the flag. This is not ideal since it leaves an
inconsistent
+# cluster, but at least verify that the flag is cleared.
+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
