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 <david@pgbackrest.org> 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
