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

On Wed, 11 Nov 2026 at 08:33, shihao zhong <zhong950419(at)gmail(dot)com> wrote:
>
> Hi Greg,
>
> > One point I'd like a reviewer to look at in 0001:
> >
> > redo now reads a block the record does not register.
>
> Could that be a torn page?
>
> > The page is not modified, so it needs no full-page image and no LSN
> > update; the old code registered it with REGBUF_NO_IMAGE, so redo already
> > worked from the on-disk copy.
>
> I think if we have a torn bucket page and an XLOG_HASH_DELETE for one
> of its overflow pages before the FPI, we may hit an issue?
>
> We do not register an FPI for it in hashbucketcleanup().
>
> if (!xlrec.is_primary_bucket_page)
> {
> uint8 flags = REGBUF_STANDARD | REGBUF_NO_IMAGE |
> REGBUF_NO_CHANGE;
>
> XLogRegisterBuffer(0, bucket_buf, flags);
> }
>
> XLogRegisterBuffer(1, buf, REGBUF_STANDARD);
>
> Then hash_xlog_delete() reads it from disk in the else branch, and
> that read fails.
>
> if (xldata->is_primary_bucket_page)
> action = XLogReadBufferForRedoExtended(record, 1, RBM_NORMAL,
> true, &deletebuf);
> else
> {
> (void) XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL,
> true, &bucketbuf);
>
> action = XLogReadBufferForRedo(record, 1, &deletebuf);
> }
>
> I don't think this is caused by your patch. This looks like an
> existing bug in master.

Thanks for the report. I was able to reproduce this on HEAD.
The attached test
(src/test/recovery/t/068a_hash_torn_primary_page_delete_first.pl) does
the following, with data checksums enabled: a) Builds a hash index
with all keys in one bucket, consisting of a primary page P and an
overflow page, and takes a checkpoint. b) Runs VACUUM to remove dead
entries from the overflow page only. This writes an XLOG_HASH_DELETE
record for the overflow page without modifying P. c) Runs a second
VACUUM to remove dead entries from P. This is the first modification
to P since the checkpoint, so the corresponding WAL record includes a
full-page image (FPI) of P. d) Crashes the server, overwrites the
second half of P in the data file with random bytes to simulate a torn
write, and restarts the server.

Crash recovery then fails while replaying the first record (the DELETE
for the overflow page), before reaching the record containing the FPI
of P:
FATAL: invalid page in block 7 of relation "base/5/16387"
CONTEXT: WAL redo at 0/017745E0 for Hash/DELETE: ... is_primary F;
blkref #1: ... blk 10 FPW

During redo, we read P only to acquire the cleanup lock, using
RBM_NORMAL. The read therefore fails checksum verification. The test
fails in the same way without the patch, where P is accessed through
block 0, and with the patch, where it is accessed through
bucket_blkno.
Since we only need P to acquire the cleanup lock and don't actually
need its contents, I think we should read it using RBM_ZERO_ON_ERROR
in hash_xlog_lock_bucket(). This would zero the buffer instead of
raising an error when a torn page is encountered, allowing the FPI
later in the WAL to restore the correct contents.

Do you think RBM_ZERO_ON_ERROR is the right approach here, or is there
a better solution to fix this?

Regards,
Vignesh