On Mon, Aug 31, 2026 at 1:37 AM Michael Paquier <[email protected]> wrote: > > On Mon, Aug 24, 2026 at 01:04:44AM +0800, Rogers Wang wrote: > > What changed since the last discussion > > -------------------------------------- > > The divergence was assessed back then as "not actively harmful". That > > no longer holds in v19: commit add323da40a added > > Assert(BufferIsDirty(vmbuffer)) to heap_xlog_prune_freeze(), which > > makes the divergence fatal on assert builds after a switchover: > > > > 1. The promoted standby has a zeroed VM, so VACUUM FREEZE sets the > > bits again, emitting PRUNE records with no FPI for the VM buffer. > > 2. The old primary, rejoined as standby, still has the bits set with > > an older page LSN, so replay takes the redo branch - but > > visibilitymap_set() is a no-op there (bits already set) and the > > buffer stays clean. > > 3. Assert(BufferIsDirty(vmbuffer)) fires, and the node crash-loops on > > that record. > > > > And even without assertions, the new database on the standby silently > > loses all all-visible/all-frozen bits and all free space data of every > > copied relation. > > Hmm. This warrants an open item for v19, at least to be discussed. > Adding Melanie in CC as the committer of add323da40a.
This part > > And even without assertions, the new database on the standby silently > > loses all all-visible/all-frozen bits and all free space data of every > > copied relation. is true completely independently of my commit. Only the assert is new and would require changes to v19. I didn't think of this way of ending up with a discrepancy between primary and standby VM. I assumed that if the VM bit was not set on the primary it would not be set on the standby. Even if we commit something to eliminate this corner case (with RelationCopyStorageUsingBuffer()), you could probably end up with a lost or truncated VM on the primary and the VM being set on the standby in some other way, so we shouldn't assume that the visibilitymap_set() will dirty the buffer. We can't set the page LSN unless we marked the buffer dirty, so for 19, I think I could just do if (BufferIsDirty()) -> PageSetLSN() In master, it probably makes sense to change back the signature of visibilitymap_set() to report if it did actually set the bit(s) and then use that to decide whether to set the page LSN. Unless we don't think it is too late in 19 to change the visibilitymap_set() API. It does mean all the other callers of visibilitymap_set() are going to discard the return value, but maybe that's okay? As for whether we should log full page images of the VM and FSM in RelationCopyStorageUsingBuffer(), that kind of seems like the right direction to go, but that doesn't feel backpatchable to me. I understand this doesn't address Matthias' concern about certain AMs, but it seems like a good idea to have the VM and FSM up-to-date after promotion and FPIs of just those forks doesn't seem like an unacceptable increase in WAL volume. I don't know if doing this will have any side effects or causes bugs in other situations, though. I'd have to think more about it... - Melanie
