Message-ID: <c881c26e-5a58-115d-4d4e-9c48a533b5d2@gmail.com>
Date: Wed, 4 Jan 2023 15:49:45 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.6.1
Subject: Re: Add a new pg_walinspect function to extract FPIs from WAL records
Content-Language: en-US
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <CALj2ACVCcvzd7WiWvD=6_7NBvVB_r6G0EGSxL4F8vosAi6Se4g@mail.gmail.com>
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: <CALj2ACVCcvzd7WiWvD=6_7NBvVB_r6G0EGSxL4F8vosAi6Se4g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/c881c26e-5a58-115d-4d4e-9c48a533b5d2%40gmail.com>
Precedence: bulk

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