Re: Add a new pg_walinspect function to extract FPIs from WAL records

Hi,

On 12/27/22 12:48 PM, Bharath Rupireddy wrote:
> Hi,
>
> Here's a patch that implements the idea of extracting full page images
> from WAL records [1] [2] with a function in pg_walinspect. This new
> function accepts start and end lsn and returns full page image info
> such as WAL record lsn, tablespace oid, database oid, relfile number,
> block number, fork name and the raw full page (as bytea). I'll
> register this in the next commitfest.
>
> Thoughts?
>

I think it makes sense to somehow align the pg_walinspect functions with the pg_waldump "features".
And since [1] added FPI "extraction" then +1 for the proposed patch in this thread.

> [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=d497093cbecccf6df26365e06a5f8f8614b591c8
> [2] https://postgr.es/m/CAOxo6XKjQb2bMSBRpePf3ZpzfNTwjQUc4Tafh21=jzjX6bX8CA@mail.gmail.com

I just have a few comments:

+
+/*
+ * Get full page images and their info associated with a given WAL record.
+ */

What about adding a few words about compression? (like "Decompression is applied if necessary"?)

+ /* Full page exists, so let's output it. */
+ if (!RestoreBlockImage(record, block_id, page))

"Full page exists, so let's output its info and content." instead?

+ <para>
+ Gets raw full page images and their information associated with all the
+ valid WAL records between <replaceable>start_lsn</replaceable> and
+ <replaceable>end_lsn</replaceable>. Returns one row per full page image.

Worth to add a few words about decompression too?

I'm also wondering if it would make sense to extend the test coverage of it (and pg_waldump) to "validate" that both
extracted images are the same and matches the one modified right after the checkpoint.

What do you think? (could be done later in another patch though).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com