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

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: Melanie Plageman <melanieplageman(at)gmail(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Alexey Makhmutov <a(dot)makhmutov(at)postgrespro(dot)ru>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Two issues leading to discrepancies in FSM data on the standby server
Date: 2026-09-07 15:22:22
Message-ID: CAPpHfdv6mC8ZH5B-+2OKjLuqpC_8H+KctMy2Caxoi5Q7_3VSFA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

On Wed, Sep 2, 2026 at 2:39 AM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
> On Tue, Sep 1, 2026 at 5:18 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > 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.
>
> Ah, I guess this all hinged on having correctly surveyed all readers
> of the FSM and ensuring they also use RBM_ZERO_ON_ERROR, which I did
> not do and it turns out they don't. Oops.
>
> > 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.
>
> I need to think more about it, but my initial thought was maybe
> RelationCopyStorage() shouldn't be reading all forks the same with a
> simple smgrread(). Though I don't know how reasonable it is to try to
> prevent anyone in the future from ever reading the FSM fork without
> RBM_ZERO_ON_ERROR. And I also didn't yet check if other callers are
> reading the FSM without RBM_ZERO_ON_ERROR already. Hmm...I'll need to
> think and dig a bit more.

I've been thinking about it for a while. While FSM code can tolerate
torn pages, other code can't (and that's not only backend code). I
think we have nothing to do with this (at least with back branches).

0001 is revert
0002 is patch for FSM readme to highlight this aspect

I'm going to push (and backpatch) 0001. 0002 needs review.

------
Regards,
Alexander Korotkov
Supabase

Attachment Content-Type Size
v1-0002-Say-in-the-FSM-README-why-its-hint-writes-still-n.patch application/x-patch 3.3 KB
v1-0001-Revert-Mark-modified-the-FSM-buffer-as-dirty-duri.patch application/x-patch 2.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2026-09-07 15:34:39 Re: Logical replication can lose an update after concurrent index invalidation
Previous Message Osama Abdul Qader 2026-09-07 15:16:50 Re: Prevent object capture in CREATE/ALTER EXTENSION scripts