| From: | Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
|---|---|
| To: | Noah Misch <noah(at)leadboat(dot)com> |
| Cc: | Melanie Plageman <melanieplageman(at)gmail(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-01 22:17:17 |
| Message-ID: | CAPpHfdtSp+svbPXfE_Jm9aOYMkuX3JhUTKgvT2WBPP1QFrDe3w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Sep 2, 2026 at 12:18 AM 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.
>
> 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.)
Apologize for not setting all the commit tags correctly. I'm going to
get into details of this issue during this week.
------
Regards,
Alexander Korotkov
Supabase
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-09-01 22:38:06 | Re: [PATCH] Speed up pg_waldump TAP test and fix some GitHub CI Windows flakiness |
| Previous Message | Alexander Korotkov | 2026-09-01 22:11:36 | Re: Function scan FDW pushdown |