Hi David,

On 15 Sep 2026, David Steele wrote:
> I left this as is from v11. I think it's a pretty normal thing to do.

I found a case where the assertion itself does not hold:

> +Assert(crc_ok &&
> +       memcmp(dataDisk, &controlData, sizeof(ControlFileData)) == 0);

During XLOG_CHECKPOINT_ONLINE replay, xlog_redo() updates
ControlFile->checkPointCopy.nextXid under ControlFileLock without
calling UpdateControlFile(). The in-memory copy can therefore differ
from the on-disk file even while we hold the lock.

I can trigger this with the v13 patch on current master: pause a
standby's base backup at basebackup-before-send-files, allocate an XID
on the primary, run CHECKPOINT and wait for replay. Resuming the backup
hits this assertion in backup_control_file().

IIUC, this mismatch does not mean that pg_control is torn. Perhaps we
could drop the whole-struct equality assertion?

On the motivation, we have had many WAL-G support requests from users
who removed backup_label to get a restored cluster to start. Here is
one example [0], and a pgBackRest case prompted by the startup hint [1].
I think protecting against this mistake is worthwhile.

One concern about describing pg_resetwal as "the only way to clear it
otherwise", even with the corruption warning: a WAL-G user actually
removed backup_label and ran pg_resetwal, thinking this only meant
giving up PITR [2]. Could we frame this as bypassing the protection,
not as a way to clear the flag? It cannot replace the WAL replay needed
to make a backup consistent.

Thank you!


Best regards, Andrey Borodin.

[0] https://github.com/wal-g/wal-g/issues/115
[1] https://github.com/pgbackrest/pgbackrest/issues/839#issuecomment-532253964
[2] https://github.com/wal-g/wal-g/issues/1279#issuecomment-1144554054



Reply via email to