Dear Michael, > 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.
Right, fixed. > 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. Indeed, added an Assert() in else part. Was it same as your expectation? > 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 :/ I could not decide that the change has more benefit than its cost, so I did nothing for it. Best Regards, Hayato Kuroda FUJITSU LIMITED https://www.fujitsu.com/
v2_add_modbuf_flag.diff
Description: v2_add_modbuf_flag.diff
