On Wed, Jan 11, 2023 at 6:32 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Tue, Jan 10, 2023 at 05:25:44PM +0100, Drouvot, Bertrand wrote: > > I like the idea of comparing the full page (and not just the LSN) but > > I'm not sure that adding the pageinspect dependency is a good thing. > > > > What about extracting the block directly from the relation file and > > comparing it with the one extracted from the WAL? (We'd need to skip the > > first 8 bytes to skip the LSN though). > > Byte-by-byte counting for the page hole? The page checksum would > matter as well, FWIW, as it is not set in WAL and a FPW logged in WAL > means that the page got modified. It means that it could have been > flushed, updating its pd_lsn and its pd_checksum on the way.
Right. LSN of FPI from the WAL record and page from the table won't be the same, essentially FPI LSN <= table page. Since the LSNs are different, checksums too. This is the reason we have masking functions common/bufmask.c and rm_mask functions defined for some of the resource managers while verifying FPI consistency in verifyBackupPageConsistency(). Note that pageinspect can give only unmasked/raw page data, which means, byte-by-byte comparison isn't possible with pageinspect too, hence I was comparing only the rows with tuple_data_split(). Therefore, reading bytes from the table page and comparing byte-by-byte with FPI requires us to invent new masking functions in the tests - simply a no-go IMO. As the concern here is to not establish pageinspect dependency with pg_waldump, I'm fine to withdraw this patch and be happy with what we have today. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
