Re: Two issues leading to discrepancies in FSM data on the standby server

On Tue, Apr 21, 2026 at 04:49:12PM +0300, Alexander Korotkov wrote:
> I also think that usage of MarkBufferDirty() here is safe. If I
> understood correctly.
> 1) When wal_log_hints = on, should be completely safe. Even if we
> have torn page after the crash, during recovery FPI from the primary
> should come first.

I think this change (commit c06d1a4) is incorrect. Assume checksums and
full-page writes are enabled, both defaults. Before this change, redo would
transition FSM pages clean->dirty only via XLogReadBufferForRedo() of an FPI.
At end of recovery, the FSM passed checksum validation. After $SUBJECT,
nothing stops the following sequence of events: finish restartpoint; dirty FSM
page via heap_xlog_*; crash tears write of that FSM page; resume recovery; end
recovery without curing the torn page. The FSM code's own use of
RBM_ZERO_ON_ERROR makes FSM code accept torn pages. Other callers assume
normal reads will succeed on FSM_FORKNUM, e.g., the
heapam_relation_copy_data() call to RelationCopyStorage(). Claude wrote a
test of that, attached.

To be fair, freespace/README itself incorrectly predicts the commit is okay,
by saying things like, "we rely on a bunch of self-correcting measures to
repair possible corruption." Long-term, we do have at least these
alternatives:

- Keep the pre-2026-05 invariant that FSM is free from torn pages at end of
recovery. (We'll likely still have the property that heap_xlog_* will read
torn FSM pages during recovery. That arises, I think, because the FPI that
fixes the torn page may be later in the WAL stream. FSM is unlike other
forks this way; other forks write before they read.)

- Require all readers of FSM_FORKNUM to use RBM_ZERO_ON_ERROR or the
equivalent.

Side excursion that I'm dumping here in case it saves someone time: I was
suspicious of the older MarkBufferDirty(), in
FreeSpaceMapPrepareTruncateRel(). Subtle interactions make it harmless. It
involves a WAL write via log_newpage_buffer(), so it almost follows the
transam/README protocol. The odd part is that the protocol-required actions
happen across two WAL records. The redo of XLOG_FPI comes first and dirties
the buffer properly. Then redo of XLOG_SMGR_TRUNCATE calls
FreeSpaceMapPrepareTruncateRel() and dirties it again. That would normally
not be okay. However, the primary side used DELAY_CHKPT_COMPLETE to prevent a
checkpoint or restartpoint between the two records. No problem behavior
arises.

> Should we push it to all supported branches?

(The commit did get back-patched, even though its commit log doesn't say that.)