Hi all, On Wed, Jul 8, 2026 at 2:49 PM David Steele <[email protected]> wrote: > > On 4/13/26 21:55, David Steele wrote: > > > > I have withdrawn this patch. If anybody wants to pick it up in the > > future I'll be happy to rebase it but I think two years is long enough > > to maintain a patch that is not getting traction. > I would like to revive this but focus on the first patch for now and > drop the second patch from consideration. > > 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 know everyone was really busy in March but now that things are a bit > calmer I'd like to revisit. > > Heikki, Robert, Andres, Fujii -- any objections or comments? I believe > Michael is on board with the feature (this part, at least) but he very > sensibly would like to have some consensus.
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 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. Regards, Solai
