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

On Wed, Jan 11, 2023 at 06:59:18PM +0530, Bharath Rupireddy wrote:
> I've done it that way for pg_get_wal_fpi_info. If this format looks
> okay, I can propose to do the same for other functions (for
> backpatching too) in a separate thread though.

My vote would be to make that happen first, to have in place cleaner
basics for the docs. I could just do it and move on..

> We deliberated to have something like below:
> pg_get_wal_stats(start_lsn, end_lsn, till_end_of_wal default false);
> pg_get_wal_records_info(start_lsn, end_lsn, till_end_of_wal default false);
>
> We wanted to have better validation of the start_lsn and end_lsn, that
> is, start_lsn < end_lsn and end_lsn mustn't fall into the future when
> users specify it by themselves (otherwise, one can easily trick the
> server by passing in the extreme end of the LSN - 0xFFFFFFFFFFFFFFFF).
> And, we couldn't find a better way to deal with when till_end_of_wal
> is passed as true (in the above version of the functions).
>
> Another idea was to have something like below:
> pg_get_wal_stats(start_lsn, end_lsn default '0/0');
> pg_get_wal_records_info(start_lsn, end_lsn default '0/0');
>
> When end_lsn is not entered or entered as invalid lsn, then return the
> stats/info till end of the WAL. Again, we wanted to have some
> validation of the user-entered end_lsn.

This reminds me of the slot advancing, where we discarded this case
just because it is useful to enforce a LSN far in the future.
Honestly, I cannot think of any case where I would use this level of
validation, especially having *two* functions to decide one behavior
or the other: this stuff could just use one function and use for
example NULL as a setup to enforce the end of WAL, on top of a LSN far
ahead.. But well..

>> be useful to make this new FPI function work at least with an insanely
>> high LSN value to make sure that we fetch all the FPIs from a given
>> start position, up to the end of WAL? That looks like a pretty good
>> default behavior to me, rather than issuing an error when a LSN is
>> defined as in the future.. I am really wondering why we have
>> ValidateInputLSNs(till_end_of_wal=false) to begin with, while we could
>> just allow any LSN value in the future automatically, as we can know
>> the current insert or replay LSNs (depending on the recovery state).
>
> Hm. How about having pg_get_wal_fpi_info_till_end_of_wal() then?

I don't really want to make the interface more bloated with more
functions than necessary, TBH, though I get your point to push for
consistency in these functions. This makes me really wonder whether
we'd better make relax all the existing functions, but I may get
outvoted.
--
Michael