Re: Add pg_walinspect function with block info columns

At Thu, 9 Mar 2023 10:15:39 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Thu, Mar 09, 2023 at 09:46:12AM +0900, Kyotaro Horiguchi wrote:
> > Although I'm not strongly opposed to pfreeing them, I'm not sure I
> > like the way the patch frees them.  The life times of all of raw_data,
> > raw_page and flags are within a block.  They can be freed
> > unconditionally after they are actually used and the scope of the
> > pointer variables can be properly narowed.
> 
> The thing is that you cannot keep them inside each individual blocks
> because they have to be freed once their values are stored in the
> tuplestore, which is why I guess Bharath has done things this way.

Ugh.. Right.

> After sleeping on that, I tend to prefer the simplicity of v3 where we
> keep track of the block and fpi data in each of their respective
> blocks.  It means that we lose track of them each time we go to a
> different block, but the memory context reset done after each record
> means that scanning through a large WAL history will not cause a leak
> across the function call.
> 
> The worst scenario with v3 is a record that makes use of all the 32
> blocks with a hell lot of block data in each one of them, which is
> possible in theory, but very unlikely in practice except if someone
> uses a custom RGMR to generate crazily-shaped WAL records.  I am aware
> of the fact that it is possible to generate such records if you are
> really willing to do so, aka this thread:
> https://www.postgresql.org/message-id/flat/CAEze2WgGiw+LZt+vHf8tWqB_6VxeLsMeoAuod0N=ij1q17n5pw@mail.gmail.com

I agree to the view that that "leakage" for at-most 32 blocks and
typically 0 to 2 blcoks won't be a matter.

> In short, my choice would still be simplicity here with v3, I guess.

FWIW, I slightly prefer v3 for the reason I mentioned above.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center