Re: Add pg_walinspect function with block info columns

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: michael@paquier.xyz
Cc: boekewurm+postgres@gmail.com, bharath.rupireddyforpostgres@gmail.com, melanieplageman@gmail.com, pgsql-hackers@postgresql.org, pg@bowt.ie
Date: 2023-03-07T02:17:45Z
Lists: pgsql-hackers
At Tue, 7 Mar 2023 09:34:24 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Mon, Mar 06, 2023 at 04:08:28PM +0100, Matthias van de Meent wrote:
> > On Mon, 6 Mar 2023 at 15:40, Bharath Rupireddy
> >> IMO, pg_get_wal_records_extended_info as proposed doesn't look good to
> >> me as it outputs most of the columns that are already given by
> >> pg_get_wal_records_info.What I think the best way at this point is to
> >> make it return the following:
> >> lsn pg_lsn
> >> block_id int8
> >> spcOid oid
> >> dbOid oid
> >> relNumber oid
> >> forkNames text
> >> fpi bytea
> >> fpi_info text
> 
> I would add the length of the block data (without the hole and
> compressed, as the FPI data should always be presented as
> uncompressed), and the block data if any (without the block data
> length as one can guess it based on the bytea data length).  Note that 
> a block can have both a FPI and some data assigned to it, as far as I
> recall.

+1

> > The basic idea is to create a single entrypoint to all relevant data
> > from DecodedXLogRecord in SQL, not multiple.
> 
> While I would agree with this principle on simplicity's ground in
> terms of minimizing the SQL interface and the pg_wal/ lookups, I
> disagree about it on unsability ground, because we can avoid extra SQL
> tweaks with more functions.  One recent example I have in mind is
> partitionfuncs.c, which can actually be achieved with a WITH RECURSIVE
> on the catalogs. There are of course various degrees of complexity,
> and perhaps unnest() cannot qualify as one, but having two functions
> returning normalized records (one for the record information, and a
> second for the block information), is a rather good balance between
> usability and interface complexity, in my experience.  If you have two
> functions, a JOIN is enough to cross-check the block data and the
> record data, while an unnest() heavily bloats the main function output
> (aka byteas of FPIs in a single array).

FWIW, my initial thought about the proposal was similar to Matthias,
and tried a function that would convert (for simplicity) the block_ref
string to a json object. Although this approach did work, I was not
satisfied with its limited usability and poor performance (mainly the
poor performance is due to text->json conversion, though)..

Finally, I realized that the initial discomfort I experienced stemmed
from the name of the function, which suggests that it returns
information of "records". This discomfort would disappear if the
function were instead named pg_get_wal_blockref_info() or something
similar.

Thus I'm inclined to agree with Michael's suggestion of creating a new
normalized set-returning function that returns information of
"blocks".

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



Commits

  1. Add show_data option to pg_get_wal_block_info.

  2. Show record information in pg_get_wal_block_info.

  3. Fix recent pg_walinspect fpi_length bug.

  4. Improve a few things in pg_walinspect

  5. Rework design of functions in pg_walinspect

  6. pg_walinspect: pg_get_wal_fpi_info() -> pg_get_wal_block_info()