On Wed, Jan 11, 2023 at 6:32 AM Michael Paquier <michael@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