pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedTwo issues leading to discrepancies in FSM data on the standby server
21+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2026-09-16 a4f18fd8f2 Say in the FSM README why its hint writes still need full page images
2026-09-08 d0518cb3e4 5d63dbd977 bee33974ee a8004e161b 89834e3f2e 35e4b760a5 3bed1f91aa Revert "Mark modified the FSM buffer as dirty during recovery"
2026-05-03 c06d1a4ba6 f5d1038d92 ca259b0840 54537de358 1cf010f216 ac3b97db38 Mark modified the FSM buffer as dirty during recovery
* Two issues leading to discrepancies in FSM data on the standby server
@ 2026-03-20 01:32 Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
0 siblings, 1 reply; 21+ messages in thread
From: Alexey Makhmutov @ 2026-03-20 01:32 UTC (permalink / raw)
To: pgsql-hackers
We’ve recently observed a situation with significant increase in
response time for insert operations after switching to a replica server.
The collected information pointed to the discrepancy in the FSM data on
the replica side, which became visible to the insert sessions once
autovacuum process pulled incorrect data from from leaf blocks into FSM
root. The entire situation was looking like the case discussed in
https://postgr.es/m/20180802172857.5skoexsilnjvgruk@alvherre.pgsql and
which was supposed to be fixed by ‘ab7dbd681’ (which introduced FSM
update during 'heap_xlog_visible' invocation). However in our case and
synthetic tests we were able to see data blocks marked as ‘all visible’,
but still having incorrect FSM records.
After analyzing the code I’ve noticed that during recovery FSM data is
updated in XLogRecordPageWithFreeSpace, which uses MarkBufferDirtyHint
to mark FSM block as modified. However, if data checksums are enabled,
then this call does nothing during recovery and is actually a no-op –
basically it just exits immediately without marking block as dirty. The
logic here is that as no new WAL data could not be generated during the
recovery, so changes to hints in block should not mark block as dirty to
avoid risk of torn pages being written. This seems logical, but it seems
not aligned well with the FSM case, as its blocks could be just zeroed
if checksum mismatch is detected. Currently changes to a FSM block could
be lost if each change to the particular FSM block occur rarely enough
to allow its eviction from the cache. To persist the change the
modification need to be performed while FSM block is still kept in
buffers and marked as dirty after receiving its FPI. If block was
already cleaned, then the change won’t be persisted and stored FSM
blocks may remain in an obsolete state. In our case the table had its
'fillfactor' parameter set below 80, so during insert bursts each FSM
block on replica side was modified only during first access of FSM block
since checkpoint (with FPI) and then by processing XLOG_HEAP2_VISIBLE
record for data once it was marked as ‘all visible’. This gives plenty
of time to cleanup buffer between these moments, so the second change
was just never written to the disk. So, large number of blocks were left
with incorrect data in FSM leaf blocks, which caused problem after
switchover.
Given that FSM is ready to handle torn page writes and
XLogRecordPageWithFreeSpace is called only during the recovery there
seems to be no reason to use MarkBufferDirtyHint here instead of a
regular MarkBufferDirty call. The code is already trying to limit
updates to the FSM (i.e. by updating it only after reaching 80% of used
space for regular DML), so we probably want to ensure that these updates
are actually persisted.
The second noticed issue (not related to our observed problem) is
related to the ‘heap_xlog_visible’ – this function uses
‘PageGetFreeSpace’ call instead of ‘PageGetHeapFreeSpace’ to get size of
free space for regular heap blocks. This seems like a bug, as method
'PageGetHeapFreeSpace' is used for any other case where we need to get
free space for a heap page. Usage of incorrect function could also cause
incorrect data being written to the FSM on replica: if block still have
free space, but already reached MaxHeapTuplesPerPage limit, then it
should be marked as unavailable for new rows in FSM, otherwise inserter
will need to check and update its FSM data as well.
Attached are separate patches, which tries to fixes both these problems
– calling ‘MarkBufferDirty’ instead of ‘MarkBufferDirtyHint’ in the
first case and replacing ‘PageGetFreeSpace’ with ‘PageGetHeapFreeSpace’
in the second case.
Two synthetic test cases are also attached which simulates both these
situations – ‘test_case1.zip’ to simulate the problem with lost FSM
update on replica side and ‘test_case2.zip’ to simulate incorrect FSM
data on standby server for blocks with large number of redirect slots.
In both cases the ‘test_prepare.sh’ script could be edited to specify
path to PG installation and port numbers. Then invoke ‘test_preapre.sh’
script to prepare two databases. For first case the second script
‘test_run.sh’ need to be invoked after that to show large number of
blocks being visited for simple insert and for second test case state of
the FSM (for single block) is just displayed at the end of
‘test_prepare.sh’.
Thanks,
Alexey
Attachments: download all (4 files, .tar.gz)
0001-Mark-modified-FSM-buffer-as-dirty-during-recovery.patch (text/x-patch, 3.3K) download
From 3a51a4f3a920bed56910ae38f1c3f12059649c56 Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Mon, 16 Mar 2026 13:32:45 +0300
Subject: [PATCH 1/2] Mark modified FSM buffer as dirty during recovery.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The XLogRecordPageWithFreeSpace function updates freespace map (FSM)
data while replaying data-level WAL records during the recovery. If FSM
block is updated, then it need to be marked as modified and currently
this task is performed using MarkBufferDirtyHint call (as in all other
cases for modifying of FSM data). However, in recovery context this
function will actually do nothing if checksums are enabled. It’s assumed
that page should not be dirtied during recovery while modifying hints to
protect from torn pages as no new WAL data could be generated at this
point to store FPI.
Such logic seems to be not fully aligned with the FSM case, as its
blocks could be just zeroed if checksum mismatch is detected. Currently
changes to a FSM block could be lost if each change to the particular
FSM block occur rarely enough to allow its eviction from the cache.
To persist the change the modification need to be performed while FSM
block is still kept in buffers and marked as dirty after receiving its
FPI. If block was already cleaned, then the change won’t be persisted
and stored FSM blocks may remain in an obsolete state.
If large number of discrepancies between data in leaf FSM blocks and
actual data blocks is accumulated on the replica server side, then this
could cause significant delays for insert operations after switchover.
Such insert operation may need to visit many data blocks marked as
having enough space in FSM only to discover that this information is
incorrect and FSM records need to be fixed. In a heavily trafficked
insert-only table with many concurrent clients performing inserts this
has been observed to cause several second stalls, causing visible
application malfunction. Desire to avoid such cases was a reason behind
the commit ab7dbd681, which introduced update of FSM data during the
heap_xlog_visible invocation. However, as update to the FSM data on the
standby side could be lost due to missing dirty flag, so there is still
a possibility of hitting such situation. Note, that having a zeroed FSM
page in such case (as result of checksum mismatch) is more preferable,
as zero value will be interpreted as indication of full data blocks and
inserter will be just routed to the next FSM block or to the end of the
table.
Given that FSM is ready to handle torn page writes and
XLogRecordPageWithFreeSpace is called only during the recovery there
seems to be no reason to use MarkBufferDirtyHint here instead of a
regular MarkBufferDirty call.
---
src/backend/storage/freespace/freespace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index b9a8f368a63..5c5d86bc106 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -232,7 +232,7 @@ XLogRecordPageWithFreeSpace(RelFileLocator rlocator, BlockNumber heapBlk,
PageInit(page, BLCKSZ, 0);
if (fsm_set_avail(page, slot, new_cat))
- MarkBufferDirtyHint(buf, false);
+ MarkBufferDirty(buf);
UnlockReleaseBuffer(buf);
}
--
2.53.0
0002-Use-PageGetHeapFreeSpace-in-heap_xlog_visible.patch (text/x-patch, 1.2K) download
From e7ba11349dfa6c0dc250562c13c8db90d53dcdab Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Mon, 16 Mar 2026 13:33:10 +0300
Subject: [PATCH 2/2] Use PageGetHeapFreeSpace in heap_xlog_visible.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Free space in regular heap pages need to be calculated using
PageGetHeapFreeSpace rather than PageGetFreeSpace. This is required to
take into account the MaxHeapTuplesPerPage limit, otherwise page may be
marked as having free space while it’s impossible to add any new row to
it.
---
src/backend/access/heap/heapam_xlog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index 1da774c1536..c8f5f4f7988 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -326,7 +326,7 @@ heap_xlog_visible(XLogReaderState *record)
if (BufferIsValid(buffer))
{
- Size space = PageGetFreeSpace(BufferGetPage(buffer));
+ Size space = PageGetHeapFreeSpace(BufferGetPage(buffer));
UnlockReleaseBuffer(buffer);
--
2.53.0
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
@ 2026-03-25 12:29 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
0 siblings, 1 reply; 21+ messages in thread
From: Andrey Borodin @ 2026-03-25 12:29 UTC (permalink / raw)
To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; +Cc: pgsql-hackers
Hi!
Very interesting cases!
> On 20 Mar 2026, at 06:32, Alexey Makhmutov <a.makhmutov@postgrespro.ru> wrote:
>
> Attached are separate patches, which tries to fixes both these problems – calling ‘MarkBufferDirty’ instead of ‘MarkBufferDirtyHint’ in the first case and replacing ‘PageGetFreeSpace’ with ‘PageGetHeapFreeSpace’ in the second case.
Patch 0001 - MarkBufferDirty() instead of MarkBufferDirtyHint() in XLogRecordPageWithFreeSpace().
Yes, MarkBufferDirtyHint() is no-op in recovery and it's the only case I found of using MarkBufferDirtyHint()
in redo.
Originally in e981653 was used MarkBufferDirty() but 96ef3b8 flipped to MarkBufferDirtyHint().
Neither of these commits provided a comment on why this version was chosen. I think if we fix it we must comment things.
Patch 0002 - PageGetHeapFreeSpace instead of PageGetFreeSpace in heap_xlog_visible.
This seems to be just an oversight in ab7dbd6. Every other call to XLogRecordPageWithFreeSpace() uses PageGetHeapFreeSpace().
And this seems correct to me, but a bit odd. Why indexes do not update FSM via this routine?
It seems indexes do not log free pages at all, relying on index vacuum to rebuild fsm on Standby.
Nice catch!
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
@ 2026-04-06 14:26 ` Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Alexey Makhmutov @ 2026-04-06 14:26 UTC (permalink / raw)
To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: pgsql-hackers
Hi Andrey!
Thank you for the attention to this patch!
> Originally in e981653 was used MarkBufferDirty() but 96ef3b8 flipped to MarkBufferDirtyHint().
> Neither of these commits provided a comment on why this version was chosen. I think if we fix it we must comment things.
I think that reason of change in 96ef3b8 (changing of 'MarkBufferDirty'
to 'MarkBufferDirtyHint') may be described in the next commit (9df56f6),
during the README update:
> New WAL records cannot be written during recovery, so hint bits set
during recovery must not dirty the page if the buffer is not already
dirty, when checksums are enabled. Systems in Hot-Standby mode may
benefit from hint bits being set, but with checksums enabled, a page
cannot be dirtied after setting a hint bit (due to the torn page risk).
So, it must wait for full-page images containing the hint bit updates to
arrive from the master.
So, it seems logical, that any changes to the data not protected by the
WAL (which includes VM and FSM as well) should use MarkBufferDirtyHint,
which does not set dirty flag during recovery. However, as FSM blocks
could be just zeroed in case of checksums mismatch, so I think it's
perfectly fine to use regular MarkBufferDirty here.
I've updated the first patch by adding the comment with explanation of
the reason for using MarkBufferDirty instead of MarkBufferDirtyHint here.
As for the second issue and the patch - it seems to be resolved in the
current master by a881cc9, which removed the entire 'heap_xlog_visible'
method, as all-visibility information is now sent with the
XLOG_HEAP2_PRUNE_VACUUM_CLEANUP message and its handler already uses
PageGetHeapFreeSpace. The problem is still relevant for the pre-19
versions, so I will probably move it to the separate thread in bugs.
Thanks,
Alexey
Attachments:
0001-Mark-modified-FSM-buffer-as-dirty-during-recovery.patch (text/x-patch, 4.0K) download
From 205e050b87870450084a55b4f42629ef27668444 Mon Sep 17 00:00:00 2001
From: Alexey Makhmutov <a.makhmutov@postgrespro.ru>
Date: Mon, 16 Mar 2026 13:32:45 +0300
Subject: [PATCH] Mark modified FSM buffer as dirty during recovery.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The XLogRecordPageWithFreeSpace function updates freespace map (FSM)
data while replaying data-level WAL records during the recovery. If FSM
block is updated, then it need to be marked as modified and currently
this task is performed using MarkBufferDirtyHint call (as in all other
cases for modifying of FSM data). However, in recovery context this
function will actually do nothing if checksums are enabled. It’s assumed
that page should not be dirtied during recovery while modifying hints to
protect from torn pages as no new WAL data could be generated at this
point to store FPI.
Such logic seems to be not fully aligned with the FSM case, as its
blocks could be just zeroed if checksum mismatch is detected. Currently
changes to a FSM block could be lost if each change to the particular
FSM block occurs rarely enough to allow its eviction from the cache.
To persist the change the modification need to be performed while FSM
block is still kept in buffers and marked as dirty after receiving its
FPI. If block was already cleaned, then the change won’t be persisted,
so stored FSM blocks may remain in an obsolete state.
If large number of discrepancies between the data in leaf FSM blocks and
actual data blocks is accumulated on the replica server side, then this
could cause significant delays for insert operations after switchover.
Such insert operation may need to visit many data blocks marked as
having enough space in FSM only to discover that this information is
incorrect and FSM records need to be fixed. In a heavily trafficked
insert-only table with many concurrent clients performing inserts this
has been observed to cause several second stalls, causing visible
application malfunction. The desire to avoid such cases was the reason
behind the commit ab7dbd681, which introduced an update of FSM data
during the heap_xlog_visible invocation. However, an update to the FSM
data on the standby side could be lost due to missing 'dirty' flag, so
there is still a possibility that a large number of FSM records will
contain incorrect data. Note, that having a zeroed FSM page in such case
(as result of checksum mismatch) is more preferable, as zero value will
be interpreted as indication of full data blocks and inserter will be
just routed to the next FSM block or to the end of the table.
Given that FSM is ready to handle torn page writes and
XLogRecordPageWithFreeSpace is called only during the recovery, there
seems to be no reason to use MarkBufferDirtyHint here instead of a
regular MarkBufferDirty call.
---
src/backend/storage/freespace/freespace.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index 40d67a96178..1631bc79872 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -231,8 +231,18 @@ XLogRecordPageWithFreeSpace(RelFileLocator rlocator, BlockNumber heapBlk,
if (PageIsNew(page))
PageInit(page, BLCKSZ, 0);
+ /*
+ * Changes to FSM are usually marked as changed using MarkBufferDirtyHint,
+ * however during recovery it does nothing if checksums are enabled. It is
+ * assumed that page should not be dirtied during recovery while modifying
+ * hints to protect from torn pages as no new WAL data could be generated
+ * at this point to store FPI. This is not relevant for the FSM case, as
+ * its blocks are just zeroed in case of checksum mismatch. So, we need to
+ * use regular MarkBufferDirty here to actually mark FSM block as modified
+ * during the recovery, otherwise changes to the FSM may be just lost.
+ */
if (fsm_set_avail(page, slot, new_cat))
- MarkBufferDirtyHint(buf, false);
+ MarkBufferDirty(buf);
UnlockReleaseBuffer(buf);
}
--
2.53.0
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
@ 2026-04-14 16:21 ` Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Melanie Plageman @ 2026-04-14 16:21 UTC (permalink / raw)
To: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; +Cc: Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Mon, Apr 6, 2026 at 10:26 AM Alexey Makhmutov
<a.makhmutov@postgrespro.ru> wrote:
>
> > Originally in e981653 was used MarkBufferDirty() but 96ef3b8 flipped to MarkBufferDirtyHint().
> > Neither of these commits provided a comment on why this version was chosen. I think if we fix it we must comment things.
>
> I think that reason of change in 96ef3b8 (changing of 'MarkBufferDirty'
> to 'MarkBufferDirtyHint') may be described in the next commit (9df56f6),
> during the README update:
> > New WAL records cannot be written during recovery, so hint bits set
> during recovery must not dirty the page if the buffer is not already
> dirty, when checksums are enabled. Systems in Hot-Standby mode may
> benefit from hint bits being set, but with checksums enabled, a page
> cannot be dirtied after setting a hint bit (due to the torn page risk).
> So, it must wait for full-page images containing the hint bit updates to
> arrive from the master.
>
> So, it seems logical, that any changes to the data not protected by the
> WAL (which includes VM and FSM as well) should use MarkBufferDirtyHint,
> which does not set dirty flag during recovery. However, as FSM blocks
> could be just zeroed in case of checksums mismatch, so I think it's
> perfectly fine to use regular MarkBufferDirty here.
Yea, I agree that this seems like simply an oversight in 96ef3b8. And
it seems safe to use MarkBufferDirty() here instead.
- Melanie
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
@ 2026-04-21 13:49 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-04-21 14:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
0 siblings, 2 replies; 21+ messages in thread
From: Alexander Korotkov @ 2026-04-21 13:49 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Tue, Apr 14, 2026 at 7:22 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> On Mon, Apr 6, 2026 at 10:26 AM Alexey Makhmutov
> <a.makhmutov@postgrespro.ru> wrote:
> >
> > > Originally in e981653 was used MarkBufferDirty() but 96ef3b8 flipped to MarkBufferDirtyHint().
> > > Neither of these commits provided a comment on why this version was chosen. I think if we fix it we must comment things.
> >
> > I think that reason of change in 96ef3b8 (changing of 'MarkBufferDirty'
> > to 'MarkBufferDirtyHint') may be described in the next commit (9df56f6),
> > during the README update:
> > > New WAL records cannot be written during recovery, so hint bits set
> > during recovery must not dirty the page if the buffer is not already
> > dirty, when checksums are enabled. Systems in Hot-Standby mode may
> > benefit from hint bits being set, but with checksums enabled, a page
> > cannot be dirtied after setting a hint bit (due to the torn page risk).
> > So, it must wait for full-page images containing the hint bit updates to
> > arrive from the master.
> >
> > So, it seems logical, that any changes to the data not protected by the
> > WAL (which includes VM and FSM as well) should use MarkBufferDirtyHint,
> > which does not set dirty flag during recovery. However, as FSM blocks
> > could be just zeroed in case of checksums mismatch, so I think it's
> > perfectly fine to use regular MarkBufferDirty here.
>
> Yea, I agree that this seems like simply an oversight in 96ef3b8. And
> it seems safe to use MarkBufferDirty() here instead.
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.
2) When wal_log_hints = off, we can end up with torn pages not covered
by FPI. Without checksums, FSM can tolerate torn pages. With
checksums, that would result in zeroed pages. FSM can tolerate that
as well. And the last shouldn't happen too frequently. So, we should
finally get way better FSM state than it is now.
Should we push it to all supported branches?
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
@ 2026-04-21 14:42 ` Melanie Plageman <melanieplageman@gmail.com>
2026-04-22 12:32 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Melanie Plageman @ 2026-04-21 14:42 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Tue, Apr 21, 2026 at 9:49 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Tue, Apr 14, 2026 at 7:22 PM Melanie Plageman
>
> > Yea, I agree that this seems like simply an oversight in 96ef3b8. And
> > it seems safe to use MarkBufferDirty() here instead.
>
> 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 FPIs from primary don't really matter here, since we are only
talking about MarkBufferDirty() in XLogRecordPageWithFreespace(). If
we change it to MarkBufferDirty() on the standby and the machine
crashes mid-write leading to a checksum error, we'll just zero it out
-- which is really your point 2. While FPIs from the primary will
overwrite the standby's FSM page, they don't provide torn-page
protection for modifications made by the standby as you could read the
page between the torn write and replaying any FPI from the primary.
> 2) When wal_log_hints = off, we can end up with torn pages not covered
> by FPI. Without checksums, FSM can tolerate torn pages. With
> checksums, that would result in zeroed pages. FSM can tolerate that
> as well. And the last shouldn't happen too frequently. So, we should
> finally get way better FSM state than it is now.
Yes, I think the bottom line is that we can't get checksum errors
reading FSM pages because of ZERO_ON_ERROR, so there is no reason to
do MarkBufferDirtyHint() in recovery for FSM. It only leads to losing
changes to the page.
> Should we push it to all supported branches?
I haven't looked at the code paths in previous versions, but as long
as they are reading FSM pages with RBM_ZERO_ON_ERROR, I think it is
safe to do so. It is a bug that is causing overly optimistic FSM
numbers, but it's not a correctness issue like wrong results/data
corruption etc. So, I think you could make an argument either way
about fixing it.
I don't know how much of Alexey's reported issue was this vs
PageGetFreeSpace() in heap_xlog_visible(). The MarkBufferDirty()
change is easy to fix, so it probably makes sense to do so. I haven't
investigated more about the PageGetFreeSpace() issue.
- Melanie
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-21 14:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
@ 2026-04-22 12:32 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-04-22 14:15 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Alexander Korotkov @ 2026-04-22 12:32 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Tue, Apr 21, 2026 at 5:42 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> On Tue, Apr 21, 2026 at 9:49 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >
> > On Tue, Apr 14, 2026 at 7:22 PM Melanie Plageman
> >
> > > Yea, I agree that this seems like simply an oversight in 96ef3b8. And
> > > it seems safe to use MarkBufferDirty() here instead.
> >
> > 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 FPIs from primary don't really matter here, since we are only
> talking about MarkBufferDirty() in XLogRecordPageWithFreespace(). If
> we change it to MarkBufferDirty() on the standby and the machine
> crashes mid-write leading to a checksum error, we'll just zero it out
> -- which is really your point 2. While FPIs from the primary will
> overwrite the standby's FSM page, they don't provide torn-page
> protection for modifications made by the standby as you could read the
> page between the torn write and replaying any FPI from the primary.
It's probably not so important in this context, but I'd like to verify
my thoughts further. My idea is that standby's changes of FSM are
mirroring primary's changes of FSM, even that FSM changes don't have
own WAL-records and being decoded from other WAL records. Thus, if
some FSM page on primary gets changed then primary emits FPI for the
first change after checkpoint. The standby restartpoints are
synchronized with primary's checkpoints, and the FSM changes mirrors
FSM changes on primary. Standby should also have its first change of
FSM page after the restartpoint covered by FPI received from primary.
So, the consistency of FSM pages should be guaranteed in the similar
way to every other WAL-logged pages, except FSM pages are not directly
WAL-logged, but got their changes decoded from main fork WAL-records.
The weak point I see in the reasoning above is the assumption that FSM
changes on standby fully mirrors FSM changes on primary. I didn't
really check this invariant. But other than that, could you please,
re-check this thoughts and let me know what do you think?
> > 2) When wal_log_hints = off, we can end up with torn pages not covered
> > by FPI. Without checksums, FSM can tolerate torn pages. With
> > checksums, that would result in zeroed pages. FSM can tolerate that
> > as well. And the last shouldn't happen too frequently. So, we should
> > finally get way better FSM state than it is now.
>
> Yes, I think the bottom line is that we can't get checksum errors
> reading FSM pages because of ZERO_ON_ERROR, so there is no reason to
> do MarkBufferDirtyHint() in recovery for FSM. It only leads to losing
> changes to the page.
>
> > Should we push it to all supported branches?
>
> I haven't looked at the code paths in previous versions, but as long
> as they are reading FSM pages with RBM_ZERO_ON_ERROR, I think it is
> safe to do so.
I've checked that since 96ef3b8 we only read FSM pages with RBM_ZERO_ON_ERROR.
> It is a bug that is causing overly optimistic FSM
> numbers, but it's not a correctness issue like wrong results/data
> corruption etc. So, I think you could make an argument either way
> about fixing it.
It has user-visible effect of increased insertion time after replica
promotion. I think this is quite a reason to backpatch.
> I don't know how much of Alexey's reported issue was this vs
> PageGetFreeSpace() in heap_xlog_visible(). The MarkBufferDirty()
> change is easy to fix, so it probably makes sense to do so. I haven't
> investigated more about the PageGetFreeSpace() issue.
Makes sense. I suggest Alexey could clarify this.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-21 14:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-22 12:32 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
@ 2026-04-22 14:15 ` Melanie Plageman <melanieplageman@gmail.com>
2026-04-23 14:17 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-30 21:57 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 2 replies; 21+ messages in thread
From: Melanie Plageman @ 2026-04-22 14:15 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Wed, Apr 22, 2026 at 8:32 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Tue, Apr 21, 2026 at 5:42 PM Melanie Plageman
> <melanieplageman@gmail.com> 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 FPIs from primary don't really matter here, since we are only
> > talking about MarkBufferDirty() in XLogRecordPageWithFreespace(). If
> > we change it to MarkBufferDirty() on the standby and the machine
> > crashes mid-write leading to a checksum error, we'll just zero it out
> > -- which is really your point 2. While FPIs from the primary will
> > overwrite the standby's FSM page, they don't provide torn-page
> > protection for modifications made by the standby as you could read the
> > page between the torn write and replaying any FPI from the primary.
>
> It's probably not so important in this context, but I'd like to verify
> my thoughts further. My idea is that standby's changes of FSM are
> mirroring primary's changes of FSM, even that FSM changes don't have
> own WAL-records and being decoded from other WAL records. Thus, if
> some FSM page on primary gets changed then primary emits FPI for the
> first change after checkpoint. The standby restartpoints are
> synchronized with primary's checkpoints, and the FSM changes mirrors
> FSM changes on primary. Standby should also have its first change of
> FSM page after the restartpoint covered by FPI received from primary.
> So, the consistency of FSM pages should be guaranteed in the similar
> way to every other WAL-logged pages, except FSM pages are not directly
> WAL-logged, but got their changes decoded from main fork WAL-records.
>
> The weak point I see in the reasoning above is the assumption that FSM
> changes on standby fully mirrors FSM changes on primary. I didn't
> really check this invariant. But other than that, could you please,
> re-check this thoughts and let me know what do you think?
Yea, I don't think they are really being mirrored. The conditions for
updating the FSM in normal operation vs in recovery are different.
When doing an INSERT, we can look for freespace on heap page 5, find
it doesn't have enough and then update the freespace map to reflect
that. Then, let's say we eventually find space for the INSERTed tuple
on heap page 10000. In normal operation, we update the FSM on
rejection, not when we find freespace. So we do not update the FSM for
heap page 10000 to reflect how much freespace it now has remaining.
Later, when replaying the INSERT, if the heuristic conditions are met
(action == BLK_NEEDS_REDO && freespace < BLCKSZ / 5) for freespace on
heap block 10000, we may then update the FSM for page 10000. The local
changes to the FSM page don't mark the buffer dirty and thus aren't
safe from loss if the page is evicted and definitely not safe from
being wiped out by a future FPI since they don't update the FSM page
LSN.
> > It is a bug that is causing overly optimistic FSM
> > numbers, but it's not a correctness issue like wrong results/data
> > corruption etc. So, I think you could make an argument either way
> > about fixing it.
>
> It has user-visible effect of increased insertion time after replica
> promotion. I think this is quite a reason to backpatch.
Right, since it's not invasive, I'm fine with backpatching.
> > I don't know how much of Alexey's reported issue was this vs
> > PageGetFreeSpace() in heap_xlog_visible(). The MarkBufferDirty()
> > change is easy to fix, so it probably makes sense to do so. I haven't
> > investigated more about the PageGetFreeSpace() issue.
>
> Makes sense. I suggest Alexey could clarify this.
Would be good also if he could add a 19 open item for the
PageGetFreeSpace() thing (in the issues affecting back branches
section) or register a patch for next commitfest (as a bug), so we
don't forget about it.
- Melanie
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-21 14:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-22 12:32 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-22 14:15 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
@ 2026-04-23 14:17 ` Alexey Makhmutov <a.makhmutov@postgrespro.ru>
1 sibling, 0 replies; 21+ messages in thread
From: Alexey Makhmutov @ 2026-04-23 14:17 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
Hi Alexander and Melanie,
Thank you very much for the attention to this thread!
On 4/22/26 17:15, Melanie Plageman wrote:
>>> I don't know how much of Alexey's reported issue was this vs
>>> PageGetFreeSpace() in heap_xlog_visible(). The MarkBufferDirty()
>>> change is easy to fix, so it probably makes sense to do so. I haven't
>>> investigated more about the PageGetFreeSpace() issue.
>> Makes sense. I suggest Alexey could clarify this.
The issue with 'PageGetFreeSpace' call is completely unrelated to the
other issue discussed in this thread (i.e. marking FSM buffer as dirty).
In my view it's just a small overlook in the ab7dbd6, which used
'PageGetFreeSpace' instead of 'PageGetHeapFreeSpace'. This latter
function calls ‘PageGetFreeSpace’ and additionally checks that there is
a space in line pointers section (i.e. checks that MaxHeapTuplesPerPage
is not exceeded). If MaxHeapTuplesPerfPage items are already created,
then free space for a heap block need to be reported as 0, as no new
tuple could be added to the page. Such case could be observed if page
has large number of redirect slots created as result of HOT cleanups.
Basically, every code working with heap pages should use
‘PageGetHeapFreeSpace’ rather than ‘PageGetFreeSpace’ and function
‘heap_xlog_visible’ is the only place in the code where
‘PageGetFreeSpace’ is used for heap pages. I think that situation with
difference between results of these two functions is rarely observed,
but it seems to be logical to just call the correct function in all cases.
This issue is present in all supported version (PG 14-18), but is not
present in current master for PG19 (as ‘heap_xlog_visible’ was removed
in a881cc9).
> Would be good also if he could add a 19 open item for the
> PageGetFreeSpace() thing (in the issues affecting back branches
> section) or register a patch for next commitfest (as a bug), so we
> don't forget about it.
Sure, I could just move this second patch (with test to reproduce the
problem) to a different thread and register a new commitfest entry for
it. Target version will be 14 (first supported version with the code).
As it cannot by applied on top of current master (as the problem was
already fixed for PG19 as result of a881cc9), so probably it will be
displayed in 'need rebase' status.
Thanks,
Alexey
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-21 14:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-22 12:32 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-22 14:15 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
@ 2026-04-30 21:57 ` Melanie Plageman <melanieplageman@gmail.com>
2026-05-02 16:51 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Melanie Plageman @ 2026-04-30 21:57 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Wed, Apr 22, 2026 at 10:15 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> On Wed, Apr 22, 2026 at 8:32 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> > > It is a bug that is causing overly optimistic FSM
> > > numbers, but it's not a correctness issue like wrong results/data
> > > corruption etc. So, I think you could make an argument either way
> > > about fixing it.
> >
> > It has user-visible effect of increased insertion time after replica
> > promotion. I think this is quite a reason to backpatch.
>
> Right, since it's not invasive, I'm fine with backpatching.
Alexander, are you planning on committing this?
- Melanie
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-21 14:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-22 12:32 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-04-22 14:15 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-30 21:57 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
@ 2026-05-02 16:51 ` Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 0 replies; 21+ messages in thread
From: Alexander Korotkov @ 2026-05-02 16:51 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Fri, May 1, 2026 at 12:57 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> On Wed, Apr 22, 2026 at 10:15 AM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> >
> > On Wed, Apr 22, 2026 at 8:32 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >
> > > > It is a bug that is causing overly optimistic FSM
> > > > numbers, but it's not a correctness issue like wrong results/data
> > > > corruption etc. So, I think you could make an argument either way
> > > > about fixing it.
> > >
> > > It has user-visible effect of increased insertion time after replica
> > > promotion. I think this is quite a reason to backpatch.
> >
> > Right, since it's not invasive, I'm fine with backpatching.
>
> Alexander, are you planning on committing this?
Melanie, thank you for reminder. Yes, I'm planning to commit (with
backpatching) this in a couple of days.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
@ 2026-09-01 21:18 ` Noah Misch <noah@leadboat.com>
2026-09-01 22:17 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
1 sibling, 2 replies; 21+ messages in thread
From: Noah Misch @ 2026-09-01 21:18 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
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.)
Attachments:
fsm-tear-test-v0.patch (text/plain, 15.5K) download
commit 0b07f8b (HEAD -> fsm-dirty-defect-tests)
Author: Noah Misch <noah@leadboat.com>
AuthorDate: Tue Sep 1 21:02:10 2026 +0000
Commit: Noah Misch <noah@leadboat.com>
CommitDate: Tue Sep 1 14:07:06 2026 -0700
recovery: focused single-defect test for the c06d1a4 torn-FSM survival
A minimal, standalone extract of the "D1 leg C" scenario already inside
057_fsm_dirty_torn.pl, for making one point without handing over the larger
multi-defect file.
On a default cluster (data checksums on, full_page_writes on, wal_log_hints
off; no online checksum enabling and no injection points), a heap page is
filled to under one FSM step of free space, then a crash tears the standby's
unprotected FSM-page write. Recovery re-records "0 bytes free" onto the
zeroed torn page, which is no change, so fsm_set_avail() never re-dirties the
buffer and the torn page survives to end of recovery. After promotion,
ALTER TABLE ... SET TABLESPACE copies the relation's forks through
RelationCopyStorage(), whose verifying reads hit the surviving torn FSM page:
ERROR: invalid page in block 2 of relation "base/5/<relfilenode>_fsm"
Since c06d1a4, XLogRecordPageWithFreeSpace() uses MarkBufferDirty() instead of
MarkBufferDirtyHint(); before it, a checksummed standby wrote no such page.
15 GUARD assertions prove the choreography (the standby wrote the page
unprotected, insert #39 carries a full-page image and #40 does not, the crash
tore the write, and the in-memory repair never reaches disk). Three CORRECT
assertions fail on master and must pass after a fix: the two in-server signals
(a startup-process WARNING and a pg_stat_database.checksum_failures bump on a
fault-free cluster) and the SET TABLESPACE ERROR. This test avoids
pg_basebackup and pg_checksums deliberately. The page self-corrects once any
nonzero free-space change touches that FSM leaf, so a fix must keep the GUARDs
passing (i.e. keep persisting replayed FSM updates).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014bw5CScTPcbfh7xU82XLmN
---
src/test/recovery/Makefile | 3 +
src/test/recovery/meson.build | 1 +
.../t/059_fsm_torn_page_survives_recovery.pl | 269 +++++++++++++++++++++
3 files changed, 273 insertions(+)
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 9c4102b..07da006 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -12,6 +12,9 @@
EXTRA_INSTALL=contrib/pg_prewarm \
contrib/pg_stat_statements \
contrib/test_decoding \
+ contrib/pg_freespacemap \
+ contrib/pg_buffercache \
+ contrib/pg_walinspect \
src/test/modules/injection_points \
src/test/modules/test_wait_lsn
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index b86fbff..8dcf200 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -67,6 +67,7 @@ tests += {
't/056_standby_snapshot_export.pl',
't/057_fsm_dirty_torn.pl',
't/058_fsm_enable_checksums.pl',
+ 't/059_fsm_torn_page_survives_recovery.pl',
],
},
}
diff --git a/src/test/recovery/t/059_fsm_torn_page_survives_recovery.pl b/src/test/recovery/t/059_fsm_torn_page_survives_recovery.pl
new file mode 100644
index 0000000..55bf810
--- /dev/null
+++ b/src/test/recovery/t/059_fsm_torn_page_survives_recovery.pl
@@ -0,0 +1,269 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# A torn free-space-map page written during recovery can survive to the end of
+# recovery, on a cluster with data checksums and full_page_writes -- both
+# defaults -- and then break a normal SQL command.
+#
+# Since commit c06d1a4, heap-record replay marks FSM pages dirty with
+# MarkBufferDirty() (was MarkBufferDirtyHint()). The FSM page is named in no
+# WAL record, so the standby writes it with a fresh checksum, pd_lsn 0, and no
+# full-page image anywhere. If a crash tears such an 8 KB write, recovery is
+# expected to cure the page. It does -- unless the heap page the FSM entry
+# describes is essentially full. Then replay re-records "0 bytes free" onto
+# the freshly-zeroed torn page, which is no change, so fsm_set_avail() does not
+# re-dirty the buffer, nothing is written back, and the torn page stays on
+# disk past the end of recovery.
+#
+# After promotion, ALTER TABLE ... SET TABLESPACE copies every fork through
+# RelationCopyStorage(), which reads with normal (verifying) reads -- unlike
+# the FSM code's own RBM_ZERO_ON_ERROR -- so it hits the surviving torn FSM
+# page and fails: ERROR: invalid page in block 2 of relation "...". No
+# hardware faulted, and before c06d1a4 a checksummed standby wrote no such page.
+#
+# The tear is simulated (a TAP test cannot cut a physical write): while the
+# server is stopped, block 2's second 4 KB (the FSM leaf nodes) is reverted to
+# its pre-write content, leaving the first 4 KB -- header, checksum, and the
+# FSM upper nodes -- as freshly written. That is byte-for-byte what a power
+# loss can persist when one 8 KB write lands as two 4 KB writes.
+#
+# "GUARD:" assertions prove the choreography really happened; they pass today.
+# "CORRECT:" assertions state what a non-defective server must do; each fails
+# on an affected build, so a failure is exactly this defect and a post-fix pass
+# is meaningful. The affected page self-corrects the moment any nonzero
+# free-space change touches that FSM leaf; this test stops before that, and a
+# fix must keep the GUARDs passing (i.e. keep persisting replayed FSM updates).
+
+use strict;
+use warnings FATAL => 'all';
+use Fcntl qw(:seek);
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+use constant BLCKSZ => 8192;
+
+# ------------------------------------------------------------------ helpers
+
+# Return the 8 KB block $blkno of $file.
+sub read_page
+{
+ my ($file, $blkno) = @_;
+ open my $fh, '<', $file or die "open $file: $!";
+ binmode $fh;
+ sysseek($fh, $blkno * BLCKSZ, SEEK_SET) // die "sysseek $file: $!";
+ my $n = sysread($fh, my $buf, BLCKSZ);
+ die "short read of $file block $blkno" unless defined $n && $n == BLCKSZ;
+ close $fh;
+ return $buf;
+}
+
+# Simulate a torn write of block $blkno: keep the first 4 KB now on disk (the
+# new write: header, pd_checksum, FSM upper nodes) and revert the second 4 KB
+# (the FSM leaf nodes) to $old_page's second half (the pre-write content).
+# The owning server must be stopped. Returns the bytes now on disk.
+sub tear_page
+{
+ my ($file, $blkno, $old_page) = @_;
+ my $cur = read_page($file, $blkno);
+ my $torn = substr($cur, 0, BLCKSZ / 2) . substr($old_page, BLCKSZ / 2);
+ open my $fh, '+<', $file or die "open $file: $!";
+ binmode $fh;
+ sysseek($fh, $blkno * BLCKSZ, SEEK_SET) // die "sysseek $file: $!";
+ my $w = syswrite($fh, $torn, BLCKSZ);
+ die "short write of $file block $blkno" unless defined $w && $w == BLCKSZ;
+ close $fh or die "close $file: $!";
+ return $torn;
+}
+
+# FSM page field accessors (see src/backend/storage/freespace/fsm_internals.h):
+# the leaf node for heap block N is at page byte 4123 + N; the root is byte 28.
+sub fsm_leaf_byte { return ord(substr($_[0], 4123 + $_[1], 1)); }
+sub fsm_root_byte { return ord(substr($_[0], 28, 1)); }
+
+# Force a restartpoint the standby cannot skip: replay a fresh primary
+# checkpoint record first, then CHECKPOINT locally.
+sub restartpoint
+{
+ my ($primary, $standby) = @_;
+ my $off = -s $standby->logfile;
+ $primary->safe_psql('postgres', 'CHECKPOINT');
+ $primary->wait_for_catchup($standby);
+ $standby->safe_psql('postgres', 'CHECKPOINT');
+ $standby->wait_for_log(qr/restartpoint complete/, $off);
+ return;
+}
+
+# pg_stat_database.checksum_failures for 'postgres'. Stats flushes are async,
+# so if the log since $off shows a verification failure, wait for the counter
+# to catch up; that way the CORRECT comparison against 0 fails deterministically
+# on an affected build instead of racing the flush.
+sub settled_checksum_failures
+{
+ my ($node, $off) = @_;
+ if ($node->log_contains(qr/page verification failed/, $off))
+ {
+ $node->poll_query_until('postgres',
+ q{SELECT coalesce(checksum_failures, 0) > 0 FROM pg_stat_database
+ WHERE datname = 'postgres'});
+ }
+ return $node->safe_psql('postgres',
+ q{SELECT coalesce(checksum_failures, 0) FROM pg_stat_database
+ WHERE datname = 'postgres'});
+}
+
+# All corruption-alarm lines in $node's log since $off ('' if none).
+sub corruption_log_lines
+{
+ my ($node, $off) = @_;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
+ return join("\n",
+ grep { /page verification failed|invalid page in block/ }
+ split(/\n/, $log));
+}
+
+# Evict FSM block 2 of $tbl; returns "buffer_evicted|buffer_flushed"
+# ('t|t' = the buffer was dirty and written; 't|f' = it was clean).
+sub evict_fsm
+{
+ my ($node, $tbl) = @_;
+ return $node->safe_psql('postgres', qq{
+SELECT buffer_evicted, buffer_flushed FROM pg_buffercache_evict(
+ (SELECT bufferid FROM pg_buffercache
+ WHERE relfilenode = pg_relation_filenode('$tbl')
+ AND reldatabase = (SELECT oid FROM pg_database
+ WHERE datname = current_database())
+ AND relforknumber = 1 AND relblocknumber = 2))});
+}
+
+# -------------------------------------------------------------------- setup
+
+my $primary = PostgreSQL::Test::Cluster->new('primary');
+$primary->init(allows_streaming => 1); # data checksums on by default
+$primary->append_conf('postgresql.conf', q{
+autovacuum = off
+wal_log_hints = off
+checkpoint_timeout = 1h
+bgwriter_lru_maxpages = 0
+});
+$primary->start;
+is($primary->safe_psql('postgres', 'SHOW data_checksums'),
+ 'on', 'GUARD: data checksums are on');
+is($primary->safe_psql('postgres', 'SHOW full_page_writes'),
+ 'on', 'GUARD: full_page_writes is on');
+$primary->safe_psql('postgres',
+ 'CREATE EXTENSION pg_freespacemap; CREATE EXTENSION pg_buffercache; '
+ . 'CREATE EXTENSION pg_walinspect;');
+$primary->backup('bk');
+
+# 32MB shared_buffers so nothing evicts a standby buffer except when we ask.
+my $standby = PostgreSQL::Test::Cluster->new('standby');
+$standby->init_from_backup($primary, 'bk', has_streaming => 1);
+$standby->append_conf('postgresql.conf', 'shared_buffers = 32MB');
+$standby->start;
+
+# --------------------------------------------------------------------- body
+
+$primary->safe_psql('postgres',
+ 'CREATE TABLE t (a int, b text) WITH (autovacuum_enabled = off)');
+restartpoint($primary, $standby);
+
+# Fill heap block 0 to 992 bytes free (stored as the FSM byte 31). The primary
+# inserter is never rejected, so the primary keeps no FSM and no WAL record
+# names the FSM fork; only the standby's replay builds one.
+$primary->safe_psql('postgres', q{DO $$ BEGIN FOR i IN 1..38 LOOP
+ INSERT INTO t VALUES (i, repeat('x', 150)); END LOOP; END $$;});
+$primary->wait_for_catchup($standby);
+is($standby->safe_psql('postgres',
+ "SELECT avail FROM pg_freespace('t') WHERE blkno = 0"),
+ '992', 'GUARD: standby records 992 bytes free for heap block 0');
+
+my $relpath = $primary->safe_psql('postgres', "SELECT pg_relation_filepath('t')");
+my $filenode = $primary->safe_psql('postgres', "SELECT pg_relation_filenode('t')");
+my $fsm = $standby->data_dir . "/${relpath}_fsm";
+note "standby FSM file: $fsm";
+
+# Land that value on disk (clean), so the later tear reverts to it.
+restartpoint($primary, $standby);
+my $old = read_page($fsm, 2);
+is(fsm_leaf_byte($old, 0), 31,
+ 'GUARD: the restartpoint wrote 31 (992 bytes free) to disk');
+
+# Insert #39 is heap block 0's first touch after that checkpoint, so it carries
+# a full-page image (restored unconditionally on replay: no FSM update).
+# Insert #40 (812 bytes) leaves 28 bytes free, under one FSM step (32 bytes),
+# so its replay lowers the FSM entry to 0 -- and carries no image.
+my $lsn_a = $primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()');
+$primary->safe_psql('postgres', "INSERT INTO t VALUES (39, repeat('x', 150))");
+$primary->wait_for_catchup($standby);
+is($standby->safe_psql('postgres',
+ "SELECT avail FROM pg_freespace('t') WHERE blkno = 0"),
+ '992', 'GUARD: the FPI-carrying insert #39 left the FSM unchanged');
+$primary->safe_psql('postgres', "INSERT INTO t VALUES (40, repeat('x', 770))");
+my $lsn_b = $primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()');
+is($primary->safe_psql('postgres', "SELECT pg_relation_size('t') / 8192"),
+ '1', 'GUARD: t is still a single heap block');
+is($primary->safe_psql('postgres', qq{
+SELECT count(*) FILTER (WHERE block_fpi_length > 0
+ AND block_fpi_info::text LIKE '%APPLY%')
+ || '|' ||
+ count(*) FILTER (WHERE coalesce(block_fpi_length, 0) = 0)
+ FROM pg_get_wal_block_info('$lsn_a', '$lsn_b')
+ WHERE relfilenode = $filenode AND relforknumber = 0}),
+ '1|1', 'GUARD: insert #39 carries a full-page image, insert #40 does not');
+$primary->wait_for_catchup($standby);
+is($standby->safe_psql('postgres',
+ "SELECT avail FROM pg_freespace('t') WHERE blkno = 0"),
+ '0', 'GUARD: insert #40 replay lowered the standby FSM entry to 0');
+
+# Write that 0 to disk the way any eviction/bgwriter would: no FPI, pd_lsn 0.
+is(evict_fsm($standby, 't'), 't|t',
+ 'GUARD: eviction flushed the replay-dirtied FSM page');
+my $written = read_page($fsm, 2);
+is(fsm_leaf_byte($written, 0), 0, 'GUARD: on-disk FSM leaf is now 0');
+is(fsm_root_byte($written), 0, 'GUARD: on-disk FSM root is now 0');
+
+# Replay a fresh primary checkpoint before the crash, so post-crash
+# restartpoints are permitted -- and shown to still not heal the page.
+$primary->safe_psql('postgres', 'CHECKPOINT');
+$primary->wait_for_catchup($standby);
+
+# Crash, and tear the 8 KB write: new first half (root 0, checksum over the new
+# page), stale second half (leaf 31, from the earlier restartpoint).
+$standby->stop('immediate');
+my $torn = tear_page($fsm, 2, $old);
+is(fsm_root_byte($torn), 0, 'GUARD: torn page root = 0 (new first half)');
+is(fsm_leaf_byte($torn, 0), 31, 'GUARD: torn page leaf = 31 (stale second half)');
+
+my $off = -s $standby->logfile;
+$standby->start;
+$primary->wait_for_catchup($standby);
+
+# Replay re-reads the torn page (RBM_ZERO_ON_ERROR) and re-records 0, which is
+# no change on the zeroed page, so the buffer is never re-dirtied.
+is(corruption_log_lines($standby, $off), '',
+ 'CORRECT: crash-restart replay raises no checksum-corruption alarm');
+is(settled_checksum_failures($standby, $off), '0',
+ 'CORRECT: checksum_failures stays 0 (there is no real corruption)');
+
+# A restartpoint now runs (the fresh primary checkpoint was re-replayed) ...
+restartpoint($primary, $standby);
+
+# ... yet the torn page still cannot be healed: the in-memory page is clean
+# (zeroed), so the repair never reaches disk.
+is(evict_fsm($standby, 't'), 't|f',
+ 'GUARD: evicting the buffer writes nothing (the repair never reached disk)');
+
+# Promote, then move the table to another tablespace. ALTER TABLE ... SET
+# TABLESPACE copies every fork through RelationCopyStorage(), whose reads
+# verify the checksum, so it hits the surviving torn FSM page and ERRORs.
+$standby->promote;
+ok($standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()'),
+ 'GUARD: standby promoted to primary');
+$standby->safe_psql('postgres',
+ "SET allow_in_place_tablespaces = true; CREATE TABLESPACE ts LOCATION ''");
+my ($ret, $out, $err) =
+ $standby->psql('postgres', 'ALTER TABLE t SET TABLESPACE ts');
+is($ret, 0, 'CORRECT: ALTER TABLE ... SET TABLESPACE succeeds');
+note "ALTER TABLE SET TABLESPACE stderr: $err" if $err ne '';
+
+done_testing();
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
@ 2026-09-01 22:17 ` Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Alexander Korotkov @ 2026-09-01 22:17 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Wed, Sep 2, 2026 at 12:18 AM Noah Misch <noah@leadboat.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
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
@ 2026-09-01 23:38 ` Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Melanie Plageman @ 2026-09-01 23:38 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Tue, Sep 1, 2026 at 5:18 PM Noah Misch <noah@leadboat.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.
- Melanie
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
@ 2026-09-07 15:22 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-08 04:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-09-12 20:05 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
0 siblings, 2 replies; 21+ messages in thread
From: Alexander Korotkov @ 2026-09-07 15:22 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Noah Misch <noah@leadboat.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
Hi!
On Wed, Sep 2, 2026 at 2:39 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> On Tue, Sep 1, 2026 at 5:18 PM Noah Misch <noah@leadboat.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
Attachments: download all (2 files, .tar.gz)
v1-0002-Say-in-the-FSM-README-why-its-hint-writes-still-n.patch (application/x-patch, 3.3K) download
From 2fba69295b831fa76f51d1380522b603c8964d2d Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Mon, 7 Sep 2026 14:50:56 +0300
Subject: [PATCH v1 2/2] Say in the FSM README why its hint writes still need
full page images
The Recovery section said we would "operate correctly without the full page
images that MarkBufferDirtyHint() provides", offering them only as a way to
lose less slot knowledge to RBM_ZERO_ON_ERROR. That is true of freespace.c,
which passes RBM_ZERO_ON_ERROR on every read, and false of the system around
it, which reads the same files and does verify checksums. Reading it as a
licence to dirty FSM pages without a full page image is what led to
c06d1a4ba6b, since reverted.
Say instead that RBM_ZERO_ON_ERROR is a property of this directory rather
than of the fork, name the readers that do not share it --
RelationCopyStorage(), RelationCopyStorageUsingBuffer(), base backups and
pg_checksums -- and note that the last two cannot be relaxed, because
a verification tool told to accept a tear we consider harmless can no longer
report one caused by failing storage.
Discussion: https://postgr.es/m/20260901211837.f6.noahmisch%40microsoft.com
---
src/backend/storage/freespace/README | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/backend/storage/freespace/README b/src/backend/storage/freespace/README
index dc2a63a137b..f272d17b050 100644
--- a/src/backend/storage/freespace/README
+++ b/src/backend/storage/freespace/README
@@ -189,9 +189,26 @@ to propagate the new free-space info into the upper pages of the FSM tree.
As a result when we write to the FSM we treat that as a hint and thus use
MarkBufferDirtyHint() rather than MarkBufferDirty(). Every read here uses
RBM_ZERO_ON_ERROR to bypass checksum mismatches and other verification
-failures. We'd operate correctly without the full page images that
-MarkBufferDirtyHint() provides, but they do decrease the chance of losing slot
-knowledge to RBM_ZERO_ON_ERROR.
+failures.
+
+That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
+itself. The files themselves are ordinary relation files, and plenty of code
+outside freespace.c reads them and does verify checksums:
+RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
+fork; the read stream in RelationCopyStorageUsingBuffer(), used by
+CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
+backups and in pg_checksums. The last two are the reason this cannot simply be
+relaxed: an external verification tool has no way to tell a tear that we
+consider harmless from one caused by failing storage, so making it accept
+the former blinds it to the latter.
+
+So the full page images MarkBufferDirtyHint() writes are not just an
+optimization that reduces how often RBM_ZERO_ON_ERROR loses knowledge.
+When checksums or wal_log_hints are enabled they are what keeps an FSM page
+that reaches disk either checksum-valid or repairable by a later FPI. Do not
+replace MarkBufferDirtyHint() with MarkBufferDirty() here on the grounds that
+the FSM survives torn pages; it does, but the rest of the system is not
+promised the same.
Relation extension is not WAL-logged. Hence, after WAL replay, an on-disk FSM
slot may indicate free space in PageIsNew() blocks that never reached disk.
--
2.55.0
v1-0001-Revert-Mark-modified-the-FSM-buffer-as-dirty-duri.patch (application/x-patch, 2.0K) download
From 3a5500a2545c87d9b36145d305c63b8ccb2e25cc Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Mon, 7 Sep 2026 14:50:08 +0300
Subject: [PATCH v1 1/2] Revert "Mark modified the FSM buffer as dirty during
recovery"
This reverts commit c06d1a4ba6b26eef27b04074683cccade6c277ee.
The commit assumed that if the FSM code tolerates torn pages, everything else
do so. Readers that do not tolerate that include RelationCopyStorage(), used
by ALTER TABLE ... SET TABLESPACE for every fork, the read stream in
RelationCopyStorageUsingBuffer() used by CREATE DATABASE ... STRATEGY =
wal_log, and, most awkwardly, the checksum verification in base backups and
pg_checksums.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20260901211837.f6.noahmisch%40microsoft.com
Backpatch-through: 14
---
src/backend/storage/freespace/freespace.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index 006edab9d77..40d67a96178 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -231,18 +231,8 @@ XLogRecordPageWithFreeSpace(RelFileLocator rlocator, BlockNumber heapBlk,
if (PageIsNew(page))
PageInit(page, BLCKSZ, 0);
- /*
- * Changes to FSM are usually marked as changed using MarkBufferDirtyHint;
- * however, during recovery, it does nothing if checksums are enabled. It
- * is assumed that the page should not be dirtied during recovery while
- * modifying hints to prevent torn pages, since no new WAL data can be
- * generated at this point to store FPI. This is not relevant to the FSM
- * case, as its blocks are zeroed when a checksum mismatch occurs. So, we
- * need to use regular MarkBufferDirty here to mark the FSM block as
- * modified during recovery, otherwise changes to the FSM may be lost.
- */
if (fsm_set_avail(page, slot, new_cat))
- MarkBufferDirty(buf);
+ MarkBufferDirtyHint(buf, false);
UnlockReleaseBuffer(buf);
}
--
2.55.0
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
@ 2026-09-08 04:42 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-09-10 22:00 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Andrey Borodin @ 2026-09-08 04:42 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Noah Misch <noah@leadboat.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; pgsql-hackers
Hi Alexander,
On Sun, Sep 7, 2026 at 3:22 PM Alexander Korotkov wrote:
> 0001 is revert
> 0002 is patch for FSM readme to highlight this aspect
>
> I'm going to push (and backpatch) 0001. 0002 needs review.
The revert looks correct to me. The heap WAL record drives the FSM
update during redo, but it contains neither a block reference nor an FPI
for the FSM page. MarkBufferDirty() therefore lets the page reach disk
without torn-page protection.
I agree with the README change. "Repairable by a later FPI" could
sound like an unrelated future FPI is expected to repair the page.
Perhaps the paragraph could instead say that, when hint changes require
WAL protection and full_page_writes is enabled, MarkBufferDirtyHint()
either provides that protection before a newly dirtied page can reach
disk or, during recovery where WAL cannot be generated, leaves a clean
page clean.
Should the README correction be backpatched with the revert? The
misleading text is present in the back branches.
The revert leaves Alexey's original performance problem open. For
comparison, the recent VM clear fix c0d9864f5ce registered VM blocks in
heap WAL records. That does not transfer cheaply to FSM, since primary
and redo do not make matching FSM updates.
For master, I think Noah's second alternative is worth developing. It
would need to be a fork-wide contract rather than scattered
ZERO_ON_ERROR exceptions: every consumer must know how to discard or
reinitialize an invalid FSM page, and verification tools need an
explicit policy. I couldn't find a concrete proposal for that in the
thread, so this seems like separate work.
There are some other cases when we try to avoid FPIs...
Or perhaps this will all become simple once we have a double-write
buffer. :)
One small commit message fix:
s/everything else do so/everything else does so/
Thank you!
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-08 04:42 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
@ 2026-09-10 22:00 ` Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 0 replies; 21+ messages in thread
From: Alexander Korotkov @ 2026-09-10 22:00 UTC (permalink / raw)
To: Andrey Borodin <x4mmm@yandex-team.ru>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Noah Misch <noah@leadboat.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; pgsql-hackers
On Tue, Sep 8, 2026 at 7:43 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
> On Sun, Sep 7, 2026 at 3:22 PM Alexander Korotkov wrote:
> > 0001 is revert
> > 0002 is patch for FSM readme to highlight this aspect
> >
> > I'm going to push (and backpatch) 0001. 0002 needs review.
>
> The revert looks correct to me. The heap WAL record drives the FSM
> update during redo, but it contains neither a block reference nor an FPI
> for the FSM page. MarkBufferDirty() therefore lets the page reach disk
> without torn-page protection.
I've pushed the revert.
> I agree with the README change. "Repairable by a later FPI" could
> sound like an unrelated future FPI is expected to repair the page.
> Perhaps the paragraph could instead say that, when hint changes require
> WAL protection and full_page_writes is enabled, MarkBufferDirtyHint()
> either provides that protection before a newly dirtied page can reach
> disk or, during recovery where WAL cannot be generated, leaves a clean
> page clean.
Didn't yet pushed the README. I wish it have more review.
> Should the README correction be backpatched with the revert? The
> misleading text is present in the back branches.
Not sure we necessarily need this. Whoever would try to fix this
should start from exploring master.
> The revert leaves Alexey's original performance problem open. For
> comparison, the recent VM clear fix c0d9864f5ce registered VM blocks in
> heap WAL records. That does not transfer cheaply to FSM, since primary
> and redo do not make matching FSM updates.
>
> For master, I think Noah's second alternative is worth developing. It
> would need to be a fork-wide contract rather than scattered
> ZERO_ON_ERROR exceptions: every consumer must know how to discard or
> reinitialize an invalid FSM page, and verification tools need an
> explicit policy. I couldn't find a concrete proposal for that in the
> thread, so this seems like separate work.
Yes, that should be separate and rather bigger work on master.
> There are some other cases when we try to avoid FPIs...
> Or perhaps this will all become simple once we have a double-write
> buffer. :)
>
> One small commit message fix:
>
> s/everything else do so/everything else does so/
That was applied before commit.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
@ 2026-09-12 20:05 ` Noah Misch <noah@leadboat.com>
2026-09-13 12:10 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Noah Misch @ 2026-09-12 20:05 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Mon, Sep 07, 2026 at 06:22:22PM +0300, Alexander Korotkov wrote:
> 0002 is patch for FSM readme to highlight this aspect
>
> I'm going to push (and backpatch) 0001. 0002 needs review.
> Subject: [PATCH v1 2/2] Say in the FSM README why its hint writes still need
> full page images
> +That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
> +itself. The files themselves are ordinary relation files, and plenty of code
> +outside freespace.c reads them and does verify checksums:
> +RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
> +fork; the read stream in RelationCopyStorageUsingBuffer(), used by
> +CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
> +backups and in pg_checksums. The last two are the reason this cannot simply be
> +relaxed: an external verification tool has no way to tell a tear that we
> +consider harmless from one caused by failing storage, so making it accept
> +the former blinds it to the latter.
One could relax the "last two" by having those tools skip checksum
verification for *_fsm files, so let's not claim it's infeasible. It's true
that this could miss failing-storage evidence that coincidentally landed in
FSM, but accepting that could be the right thing someday. Other than that, I
agree with this README change. Thanks.
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-12 20:05 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
@ 2026-09-13 12:10 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-13 17:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 21+ messages in thread
From: Alexander Korotkov @ 2026-09-13 12:10 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Sat, Sep 12, 2026 at 11:05 PM Noah Misch <noah@leadboat.com> wrote:
> On Mon, Sep 07, 2026 at 06:22:22PM +0300, Alexander Korotkov wrote:
> > 0002 is patch for FSM readme to highlight this aspect
> >
> > I'm going to push (and backpatch) 0001. 0002 needs review.
>
> > Subject: [PATCH v1 2/2] Say in the FSM README why its hint writes still need
> > full page images
>
> > +That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
> > +itself. The files themselves are ordinary relation files, and plenty of code
> > +outside freespace.c reads them and does verify checksums:
> > +RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
> > +fork; the read stream in RelationCopyStorageUsingBuffer(), used by
> > +CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
> > +backups and in pg_checksums. The last two are the reason this cannot simply be
> > +relaxed: an external verification tool has no way to tell a tear that we
> > +consider harmless from one caused by failing storage, so making it accept
> > +the former blinds it to the latter.
>
> One could relax the "last two" by having those tools skip checksum
> verification for *_fsm files, so let's not claim it's infeasible. It's true
> that this could miss failing-storage evidence that coincidentally landed in
> FSM, but accepting that could be the right thing someday. Other than that, I
> agree with this README change. Thanks.
I've corrected that paragraph. I'm going to push this to master if no
objections.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
v2-0001-Say-in-the-FSM-README-why-its-hint-writes-still-n.patch (application/octet-stream, 3.5K) download
From 95780a8daef333c2c753cda17f2f9f5ae7cf079c Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Sun, 13 Sep 2026 15:09:24 +0300
Subject: [PATCH v2] Say in the FSM README why its hint writes still need full
page images
The Recovery section said we would "operate correctly without the full page
images that MarkBufferDirtyHint() provides", offering them only as a way to
lose less slot knowledge to RBM_ZERO_ON_ERROR. That is true of freespace.c,
which passes RBM_ZERO_ON_ERROR on every read, and false of the system around
it, which reads the same files and does verify checksums. Reading it as a
licence to dirty FSM pages without a full page image is what led to
c06d1a4ba6b, since reverted.
Say instead that RBM_ZERO_ON_ERROR is a property of this directory rather
than of the fork, and name the readers that do not share it:
RelationCopyStorage(), RelationCopyStorageUsingBuffer(), base backups and
pg_checksums. Note that the last two could be taught to skip the FSM
instead, at the cost of no longer reporting failing storage that lands
there, rather than calling that impossible.
Reported-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Noah Misch <noah@leadboat.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/20260901211837.f6.noahmisch%40microsoft.com
---
src/backend/storage/freespace/README | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/backend/storage/freespace/README b/src/backend/storage/freespace/README
index dc2a63a137b..b8b2c2784d5 100644
--- a/src/backend/storage/freespace/README
+++ b/src/backend/storage/freespace/README
@@ -189,9 +189,27 @@ to propagate the new free-space info into the upper pages of the FSM tree.
As a result when we write to the FSM we treat that as a hint and thus use
MarkBufferDirtyHint() rather than MarkBufferDirty(). Every read here uses
RBM_ZERO_ON_ERROR to bypass checksum mismatches and other verification
-failures. We'd operate correctly without the full page images that
-MarkBufferDirtyHint() provides, but they do decrease the chance of losing slot
-knowledge to RBM_ZERO_ON_ERROR.
+failures.
+
+That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
+itself. The files themselves are ordinary relation files, and plenty of code
+outside freespace.c reads them and does verify checksums:
+RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
+fork; the read stream in RelationCopyStorageUsingBuffer(), used by
+CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
+backups and in pg_checksums. The last two could instead be taught to skip
+*_fsm files, and that may be the right call someday. The cost would be that
+a tool told to ignore a tear in the FSM can no longer report one caused by
+failing storage. Today they do verify, so what we write here has to be
+valid.
+
+So the full page images MarkBufferDirtyHint() writes are not just an
+optimization that reduces how often RBM_ZERO_ON_ERROR loses knowledge.
+When checksums or wal_log_hints are enabled they are what keeps an FSM page
+that reaches disk either checksum-valid or repairable by a later FPI. Do not
+replace MarkBufferDirtyHint() with MarkBufferDirty() here on the grounds that
+the FSM survives torn pages; it does, but the rest of the system is not
+promised the same.
Relation extension is not WAL-logged. Hence, after WAL replay, an on-disk FSM
slot may indicate free space in PageIsNew() blocks that never reached disk.
--
2.55.0
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-12 20:05 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-13 12:10 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
@ 2026-09-13 17:21 ` Noah Misch <noah@leadboat.com>
2026-09-16 10:51 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Noah Misch @ 2026-09-13 17:21 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Sun, Sep 13, 2026 at 03:10:58PM +0300, Alexander Korotkov wrote:
> On Sat, Sep 12, 2026 at 11:05 PM Noah Misch <noah@leadboat.com> wrote:
> > On Mon, Sep 07, 2026 at 06:22:22PM +0300, Alexander Korotkov wrote:
> > > 0002 is patch for FSM readme to highlight this aspect
> > >
> > > I'm going to push (and backpatch) 0001. 0002 needs review.
> >
> > > Subject: [PATCH v1 2/2] Say in the FSM README why its hint writes still need
> > > full page images
> >
> > > +That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
> > > +itself. The files themselves are ordinary relation files, and plenty of code
> > > +outside freespace.c reads them and does verify checksums:
> > > +RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
> > > +fork; the read stream in RelationCopyStorageUsingBuffer(), used by
> > > +CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
> > > +backups and in pg_checksums. The last two are the reason this cannot simply be
> > > +relaxed: an external verification tool has no way to tell a tear that we
> > > +consider harmless from one caused by failing storage, so making it accept
> > > +the former blinds it to the latter.
> >
> > One could relax the "last two" by having those tools skip checksum
> > verification for *_fsm files, so let's not claim it's infeasible. It's true
> > that this could miss failing-storage evidence that coincidentally landed in
> > FSM, but accepting that could be the right thing someday. Other than that, I
> > agree with this README change. Thanks.
>
> I've corrected that paragraph. I'm going to push this to master if no
> objections.
> +That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
> +itself. The files themselves are ordinary relation files, and plenty of code
> +outside freespace.c reads them and does verify checksums:
> +RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
> +fork; the read stream in RelationCopyStorageUsingBuffer(), used by
> +CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
> +backups and in pg_checksums. The last two could instead be taught to skip
> +*_fsm files, and that may be the right call someday. The cost would be that
> +a tool told to ignore a tear in the FSM can no longer report one caused by
> +failing storage. Today they do verify, so what we write here has to be
> +valid.
I'd handle the last sentences differently, yielding a paragraph like this:
That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
itself. The files themselves are ordinary relation files, and plenty of code
outside freespace.c reads them and does verify checksums:
RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
fork; the read stream in RelationCopyStorageUsingBuffer(), used by CREATE
DATABASE ... STRATEGY = wal_log; and the checksum verification in base backups
and in pg_checksums. The alternative would be all such code skipping checksum
validation of *_fsm files, and that may be the right call someday. Today they
do verify, so what we write here has to be valid.
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: Two issues leading to discrepancies in FSM data on the standby server
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Re: Two issues leading to discrepancies in FSM data on the standby server Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-01 23:38 ` Re: Two issues leading to discrepancies in FSM data on the standby server Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-12 20:05 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
2026-09-13 12:10 ` Re: Two issues leading to discrepancies in FSM data on the standby server Alexander Korotkov <aekorotkov@gmail.com>
2026-09-13 17:21 ` Re: Two issues leading to discrepancies in FSM data on the standby server Noah Misch <noah@leadboat.com>
@ 2026-09-16 10:51 ` Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 0 replies; 21+ messages in thread
From: Alexander Korotkov @ 2026-09-16 10:51 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Alexey Makhmutov <a.makhmutov@postgrespro.ru>; Andrey Borodin <x4mmm@yandex-team.ru>; pgsql-hackers
On Sun, Sep 13, 2026 at 8:21 PM Noah Misch <noah@leadboat.com> wrote:
> On Sun, Sep 13, 2026 at 03:10:58PM +0300, Alexander Korotkov wrote:
> > On Sat, Sep 12, 2026 at 11:05 PM Noah Misch <noah@leadboat.com> wrote:
> > > On Mon, Sep 07, 2026 at 06:22:22PM +0300, Alexander Korotkov wrote:
> > > > 0002 is patch for FSM readme to highlight this aspect
> > > >
> > > > I'm going to push (and backpatch) 0001. 0002 needs review.
> > >
> > > > Subject: [PATCH v1 2/2] Say in the FSM README why its hint writes still need
> > > > full page images
> > >
> > > > +That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
> > > > +itself. The files themselves are ordinary relation files, and plenty of code
> > > > +outside freespace.c reads them and does verify checksums:
> > > > +RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
> > > > +fork; the read stream in RelationCopyStorageUsingBuffer(), used by
> > > > +CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
> > > > +backups and in pg_checksums. The last two are the reason this cannot simply be
> > > > +relaxed: an external verification tool has no way to tell a tear that we
> > > > +consider harmless from one caused by failing storage, so making it accept
> > > > +the former blinds it to the latter.
> > >
> > > One could relax the "last two" by having those tools skip checksum
> > > verification for *_fsm files, so let's not claim it's infeasible. It's true
> > > that this could miss failing-storage evidence that coincidentally landed in
> > > FSM, but accepting that could be the right thing someday. Other than that, I
> > > agree with this README change. Thanks.
> >
> > I've corrected that paragraph. I'm going to push this to master if no
> > objections.
>
> > +That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
> > +itself. The files themselves are ordinary relation files, and plenty of code
> > +outside freespace.c reads them and does verify checksums:
> > +RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
> > +fork; the read stream in RelationCopyStorageUsingBuffer(), used by
> > +CREATE DATABASE ... STRATEGY = wal_log; and the checksum verification in base
> > +backups and in pg_checksums. The last two could instead be taught to skip
> > +*_fsm files, and that may be the right call someday. The cost would be that
> > +a tool told to ignore a tear in the FSM can no longer report one caused by
> > +failing storage. Today they do verify, so what we write here has to be
> > +valid.
>
> I'd handle the last sentences differently, yielding a paragraph like this:
>
> That RBM_ZERO_ON_ERROR is a property of this directory, not of the FSM fork
> itself. The files themselves are ordinary relation files, and plenty of code
> outside freespace.c reads them and does verify checksums:
> RelationCopyStorage(), which ALTER TABLE ... SET TABLESPACE runs over every
> fork; the read stream in RelationCopyStorageUsingBuffer(), used by CREATE
> DATABASE ... STRATEGY = wal_log; and the checksum verification in base backups
> and in pg_checksums. The alternative would be all such code skipping checksum
> validation of *_fsm files, and that may be the right call someday. Today they
> do verify, so what we write here has to be valid.
Pushed to master with your corrections.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 21+ messages in thread
end of thread, other threads:[~2026-09-16 10:51 UTC | newest]
Thread overview: 21+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-20 01:32 Two issues leading to discrepancies in FSM data on the standby server Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-03-25 12:29 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-04-06 14:26 ` Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-14 16:21 ` Melanie Plageman <melanieplageman@gmail.com>
2026-04-21 13:49 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-04-21 14:42 ` Melanie Plageman <melanieplageman@gmail.com>
2026-04-22 12:32 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-04-22 14:15 ` Melanie Plageman <melanieplageman@gmail.com>
2026-04-23 14:17 ` Alexey Makhmutov <a.makhmutov@postgrespro.ru>
2026-04-30 21:57 ` Melanie Plageman <melanieplageman@gmail.com>
2026-05-02 16:51 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 21:18 ` Noah Misch <noah@leadboat.com>
2026-09-01 22:17 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-01 23:38 ` Melanie Plageman <melanieplageman@gmail.com>
2026-09-07 15:22 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-08 04:42 ` Andrey Borodin <x4mmm@yandex-team.ru>
2026-09-10 22:00 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-12 20:05 ` Noah Misch <noah@leadboat.com>
2026-09-13 12:10 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-13 17:21 ` Noah Misch <noah@leadboat.com>
2026-09-16 10:51 ` Alexander Korotkov <aekorotkov@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox