Hi Solai,
Thank you for having a look at the patch.
On 7/14/26 17:27, solai v wrote:
On Wed, Jul 8, 2026 at 2:49 PM David Steele <[email protected]> wrote:
The patch now implements only the new flag for pgcontrol and wires this
logic into basebackup. It has the additional benefit of guaranteeing
that the base backup contains a non-torn version of pgcontrol.
I reviewed and tested this patch on my PostgreSQL 20devel tree. The
patch applied cleanly and built successfully without any issues. As
part of the testing, I first attempted to reproduce the issue using a
backup created with pg_backup_start()/pg_backup_stop() followed by a
filesystem copy of the data directory. After restoring the backup and
deleting backup_label, the server started successfully.
While in between, I found that backupLabelRequired is set through
backup_control_file(), due to which the above workflow does not
exercise the new code path introduced by this patch.
I previously had a second patch that would also make
pg_backup_start()/pg_backup_stop() work with this new feature but
Michael did not seem in favor of it so I dropped it to try and get the
first part committed.
I then repeated
the test using pg_basebackup. After creating a physical backup, I
verified using pg_controldata that the backed-up pg_control contained:
Backup label required: yes.
I continued with the testing like restoring the backup, removing the
backup_label file, and attempting to start the restored cluster. The
PostgreSQL refused to start and reported the errors as expected:
FATAL: could not find backup_label required for recovery
HINT: restore the backup_label file that was created during the backup.
This behavior is consistent with the intended purpose of the patch and
discussions thereby preventing recovery from a pg_basebackup backup
when the required backup_label file is missing. And also I reviewed
the implementation and verified that the backupLabelRequired field is
added to ControlFileData, set while preparing the copied pg_control in
backup_control_file(), checked during recovery, and cleared after
successful recovery. Overall, the patch behaves as intended and looks
good to me.
Excellent, thank you!
Regards,
-David
