On 10/23/26 17:47, Heikki Linnakangas wrote:
On 23/10/2026 15:42, David Steele wrote:
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.
+0.5 for following the example of c558e6fd92f. This isn't performance-
sensitive so that aspect doesn't matter. But it seems a little
surprising if a backup might have more up-to-date values of some fields
than what the SQL functions show.
Another +0.5 from me makes it +1. We know that pg_control in memory can
be ahead of pg_control on disk (for standbys) and we are quite sure that
is not an issue but it seems better not to take the risk for something
that we are going to back patch. Also better to use the same mechanism
everywhere.
I've attached a patch 01 that does that and rebased 02 on it. 02 does
not make any of the changes that you requested in your later message but
I'll take care of those separately. Since minor releases are coming up
it seems better to prioritize 01.
Note that back patching this below PG17 will require also back patching
7606175991f (see commit message for details).
Regards,
-David
From c0a88ffad9ee92757e6ef2e7a806b8d73b77da5f Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Sat, 24 Oct 2026 15:59:45 +0000
Subject: Avoid a torn copy of pg_control in base backups.
Base backups read pg_control from disk without any protection against
concurrent writes by the server. 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.
Read pg_control while holding ControlFileLock, as commit c558e6fd92f did for
the SQL functions that read pg_control. The file is read into memory and sent
from there so the lock is not held while the contents are streamed to the
client. Commit 63a582222c6 dealt with the same hazard in frontend tools by
retrying the read until the CRC is stable, but a base backup runs in the
server and can use ControlFileLock instead.
Since the read is done under the lock, a CRC mismatch means that pg_control
is actually corrupt so raise an error rather than include it in the backup.
Backups taken with the low-level API still copy pg_control from disk and are
not affected by this change.
Back-patch to all supported branches. Before v17, sendFileWithContent() gets
the length of the content with strlen(), which would truncate pg_control at
its first zero byte. Commit 7606175991f, which added the length argument, must
be back-patched first. It is confined to basebackup.c where the function is
static.
---
src/backend/backup/basebackup.c | 35 ++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index d7c485dc002..b5740f758dd 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -23,9 +23,11 @@
#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"
+#include "common/controldata_utils.h"
#include "common/file_perm.h"
#include "common/file_utils.h"
#include "lib/stringinfo.h"
@@ -42,6 +44,7 @@
#include "storage/checksum.h"
#include "storage/dsm_impl.h"
#include "storage/ipc.h"
+#include "storage/lwlock.h"
#include "storage/reinit.h"
#include "utils/builtins.h"
#include "utils/guc.h"
@@ -340,9 +343,11 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
if (ti->path == NULL)
{
- struct stat statbuf;
bool sendtblspclinks = true;
char *backup_label;
+ ControlFileData *controlData;
+ bool crc_ok;
+ char
controlFile[PG_CONTROL_FILE_SIZE];
bbsink_begin_archive(sink, "base.tar");
@@ -364,15 +369,27 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
sendDir(sink, ".", 1, false, state.tablespaces,
sendtblspclinks, &manifest,
InvalidOid, ib);
- /* ... and pg_control after everything else. */
- if (lstat(XLOG_CONTROL_FILE, &statbuf) != 0)
+ /*
+ * ... and pg_control after everything else.
Read it while
+ * holding ControlFileLock so that a concurrent
write cannot
+ * produce a torn copy.
+ */
+ LWLockAcquire(ControlFileLock, LW_SHARED);
+ controlData = get_controlfile(DataDir, &crc_ok);
+ LWLockRelease(ControlFileLock);
+
+ if (!crc_ok)
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);
+ (errmsg("calculated CRC
checksum does not match value stored in file")));
+
+ /* Send the full file, zero-padded as it is on
disk */
+ memset(controlFile, 0, PG_CONTROL_FILE_SIZE);
+ memcpy(controlFile, controlData,
sizeof(ControlFileData));
+ pfree(controlData);
+
+ sendFileWithContent(sink, XLOG_CONTROL_FILE,
+
controlFile, PG_CONTROL_FILE_SIZE,
+
&manifest);
}
else
{
--
2.34.1
From a715fdb1ec2990a151ce456adb9d9b3ae1d79cd9 Mon Sep 17 00:00:00 2001
From: David Steele <[email protected]>
Date: Sat, 24 Oct 2026 15:59:45 +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/xlogrecovery.c | 27 ++-
src/backend/backup/basebackup.c | 8 +
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, 270 insertions(+), 6 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/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/backup/basebackup.c b/src/backend/backup/basebackup.c
index b5740f758dd..688f65c9405 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -382,6 +382,14 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
ereport(ERROR,
(errmsg("calculated CRC
checksum does not match value stored in file")));
+ /* Require backup_label for recovery from this
backup */
+ controlData->backupLabelRequired = true;
+
+ INIT_CRC32C(controlData->crc);
+ COMP_CRC32C(controlData->crc, controlData,
+
offsetof(ControlFileData, crc));
+ FIN_CRC32C(controlData->crc);
+
/* Send the full file, zero-padded as it is on
disk */
memset(controlFile, 0, PG_CONTROL_FILE_SIZE);
memcpy(controlFile, controlData,
sizeof(ControlFileData));
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
