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

On Thu, Jan 12, 2023 at 05:37:40PM +0530, Bharath Rupireddy wrote:
> I understand. I don't mind discussing something like [1] with the
> following behaviour and discarding till_end_of_wal functions
> altogether:
> If start_lsn is NULL, error out/return NULL.
> If end_lsn isn't specified, default to NULL, then determine the end_lsn.
> If end_lsn is specified as NULL, then determine the end_lsn.
> If end_lsn is specified as non-NULL, then determine if it is greater
> than start_lsn if yes, go ahead do the job, otherwise error out.
>
> I'll think a bit more on this and perhaps discuss it separately.

FWIW, I still find the current interface of the module bloated. So,
while it is possible to stick some pg_current_wal_lsn() calls to
bypass the error in most cases, enforcing the end of WAL with a NULL
or larger value would still be something I would push for based on my
own experience as there would be no need to worry about the latest LSN
as being two different values in two function contexts. You could
keep the functions as STRICT for consistency, and just allow larger
values as a synonym for the end of WAL.

Saying that, the version of pg_get_wal_fpi_info() committed respects
the current behavior of the module, with an error on an incorrect end
LSN.

> I'll keep the FPI extract function simple as proposed in the patch and
> I'll not go write till_end_of_wal version. If needed to get all the
> FPIs till the end of WAL, one can always determine the end_lsn with
> pg_current_wal_flush_lsn()/pg_last_wal_replay_lsn() when in recovery,
> and use the proposed function.

I was reading the patch this morning, and that's pretty much what I
would have done in terms of simplicity with a test checking that at
least one FPI has been generated. I have shortened a bit the
documentation, tweaked a few comments and applied the whole after
seeing the result.

One thing that I have been wondering about is whether it is worth
adding the block_id from the record in the output, but discarded this
idea as it could be confused with the physical block number, even if
this function is for advanced users.
--
Michael