Horton

Re: Is this a problem in GenericXLogFinish()?

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Heikki Linnakangas <hlinnaka@iki.fi>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2024-04-09T06:53:08Z
Lists: pgsql-hackers 
On Fri, Apr 05, 2024 at 06:22:58AM +0000, Hayato Kuroda (Fujitsu) wrote:
> Thanks for pointing out. Yes, we fixed a behavior by the commit aa5edbe37,
> but we missed the redo case. I made a fix patch based on the suggestion [1].

+       bool        mod_buf = false;

Perhaps you could name that mod_wbuf, to be consistent with the WAL
insert path.

I'm slightly annoyed by the fact that there is no check on
is_prim_bucket_same_wrt for buffer 1 in the BLK_NEEDS_REDO case to
show the symmetry between the insert and replay paths.  Shouldn't
there be at least an assert for that in the branch when there are no
tuples (imagine an else to cover xldata->ntups == 0)?  I mean between
just before updating the hash page's opaque area when
is_prev_bucket_same_wrt.

I've been thinking about ways to make such cases detectable in an
automated fashion.  The best choice would be
verifyBackupPageConsistency(), just after RestoreBlockImage() on the
copy of the block image stored in WAL before applying the page masking
which would mask the LSN.  A problem, unfortunately, is that we would
need to transfer the knowledge of REGBUF_NO_CHANGE as a new BKPIMAGE_*
flag so we would be able to track if the block rebuilt from the record
has the *same* LSN as the copy used for the consistency check.  So
this edge consistency case would come at a cost, I am afraid, and the
8 bits of bimg_info are precious :/
--
Michael

Commits

  1. Fix inconsistency with replay of hash squeeze record for clean buffers

  2. Set LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.

  3. Fix an uninitialized access in hash_xlog_squeeze_page().

  4. Use REGBUF_NO_CHANGE at one more place in the hash index.

  5. Assert that buffers are marked dirty before XLogRegisterBuffer().

  6. Fix bug in GenericXLogFinish().

  7. Improve hash index bucket split behavior.