Re: Hash index: stop registering pin-only bucket pages in WAL (remove REGBUF_NO_CHANGE)

On Thu, 12 Nov 2026 at 02:41, Greg Burd <greg(at)burd(dot)me> wrote:
>
> Hello,
>
> Following up on the torn-page problem Shihao found and Vignesh
> reproduced, and on Amit's comments. I reproduced it, tested the fixes,
> and have reworked the series so the torn-page fix comes first.
>
> The case of the torn-page: The bug is on master today, independent of
> the pin-only rework. I ran Vignesh's reproducer (adapted so it finds
> the primary bucket page from a record that registers it, rather than
> from the removed bucket_blkno field) against three trees:
>
> - plain master: recovery fails replaying the overflow DELETE, "invalid
> page in block 7", before the later full-page image of block 7.
> - the pin-only series: same failure, reached through the appended
> block number instead of block 0.
> - master + the fix: recovery succeeds and the index returns the right
> rows.
>
> The hazard was spotted during the original hash WAL work. In 2017
> Robert raised it on the write-ahead-logging thread [1]:
>
> "the page is registered and therefore an FPI will be taken if this is
> the first reference to this page since the checkpoint, but
> hash_xlog_delete won't restore the FPI. I think that exposes us to a
> torn-page hazard. _hash_freeovflpage/hash_xlog_squeeze_page seems to
> have the same disease, as does
> _hash_squeezebucket/hash_xlog_move_page_contents."
>
> Redo was then switched to XLogReadBufferForRedoExtended() so it restores
> an image when the record has one, but these records register the primary
> bucket page pin-only with REGBUF_NO_IMAGE, so there is nothing to
> restore and the RBM_NORMAL read still reaches the torn page. So the
> hazard stayed open; Shihao and Vignesh have now hit it.
>
> On the fix: RBM_ZERO_ON_ERROR (Vignesh) does fix the reproducer, but as
> Amit noted it is documented for non-critical forks where the caller
> repairs, and this is a main-fork index page. Guarding the read with
> InHotStandby (Amit's suggestion) also fixes it and is the right one:
>
> - The read exists only to take the cleanup lock, which only matters to
> serialize against a scan on a hot standby. When hot standby is not
> active no such scan can exist, so neither the read nor the lock is
> needed.
> - That is exactly the window where the torn-page hazard lives. The
> repairing image always precedes the point where hot standby becomes
> active, so once InHotStandby is true any earlier image has already
> been replayed and the page is no longer torn.
> - It matches existing practice: nbtree and gist already gate their
> redo cleanup-lock work on InHotStandby.
>
> Since the bug is pre-existing, the fix is patch 0001: it stands on its
> own against master, applies with or without the rework, and (the
> back-branch question aside) is separate from the refactor.

The fix skips reading and acquiring the cleanup lock on the primary
bucket page unless 'InHotStandby' is true. This works for crash
recovery, and the first test (068a) passes. However, 'InHotStandby'
does not mean that hot standby is ready to accept queries. It becomes
true as soon as the first 'XLOG_RUNNING_XACTS' record is replayed,
which happens shortly after the redo pointer is reached, well before
recovery reaches consistency. With 'hot_standby = on', redo can
therefore still read the page during this window.

A base backup can contain a torn copy of the page if it is written
while the backup files are being copied. The full-page image for that
page is then present in WAL generated after the backup starts. The
DELETE record for the overflow page can precede that image, while the
standby reaches consistency only after the backup ends, by which time
the image should have been replayed.

The attached test (068b) reproduces this scenario: a) Takes a
low-level backup using 'pg_backup_start()'. b) While the backup is in
progress, runs VACUUM to generate a DELETE record for the overflow
page, followed by another VACUUM that modifies the primary page for
the first time since the backup's checkpoint. The latter record
therefore includes a full-page image of the primary page. c) Tears the
backup copy of the primary page. d) Starts a standby with 'hot_standby
= on' using that backup.

With the fix, the standby fails to start:
FATAL: invalid page in block 7 of relation "base/5/16387"
CONTEXT: WAL redo at 0/020099A0 for Hash/DELETE: ... is_primary F;
blkref #0: ... blk 7; blkref #1: ... blk 10 FPW

The attached patch (0001) uses 'HotStandbyActiveInReplay()' instead.
This becomes true only after recovery has reached consistency and the
postmaster has been notified that connections can be accepted. Since
no scans can be running before that point, there is no need to read or
lock the primary bucket page earlier. By then, any full-page image
needed to repair a torn page should also have been replayed.
How about changing InHotStandby to HotStandbyActiveInReplay to fix this issue?

Regards,
Vignesh