Re: Is this a problem in GenericXLogFinish()?

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