Hi Andrey,
Thank you for having a look at the patch.
On 9/15/26 21:13, Andrey Borodin wrote:
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?
The mismatch is not an issue as far as I can see. I'll leave that
determination up to Michael, though, since he added the assert.
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.
I've seen this now more times than I can count, even from people that I
would expect to know better. The hint from Postgres to delete
backup_label certainly does not help the situation, though even with
this patch we can't remove it completely. I'm hoping to at least deal
with the common case.
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.
Yeah, I have also seen pg_resetwal abused in various ways. I added that
language based on feedback from Shihao and I think it should be
documented but perhaps we need more cautionary language there. Let's see
what people think.
Regards,
-David
