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.
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
In short, my choice would still be simplicity here with v3, I guess.
--
Michael
