Re: WAL_LOG CREATE DATABASE strategy broken for non-standard page layouts

On Mon, Aug 31, 2026 at 1:37 AM Michael Paquier <michael(at)paquier(dot)xyz> 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