Re: Extending SMgrRelation lifetimes

On Wed, Aug 16, 2023 at 4:11 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> Makes sense.

Thanks for looking!

> If you change smgrclose() to do what smgrrelease() does now, then it
> will apply automatically to extensions.
>
> If an extension is currently using smgropen()/smgrclose() correctly,
> this patch alone won't make it wrong, so it's not very critical for
> extensions to adopt the change. However, if after this we consider it OK
> to hold a pointer to SMgrRelation for longer, and start doing that in
> the backend, then extensions need to be adapted too.

Yeah, that sounds quite compelling.  Let's try that way:

 * smgrrelease() is removed
 * smgrclose() now releases resources, but doesn't destroy until EOX
 * smgrdestroy() now frees memory, and should rarely be used

Still WIP while I think about edge cases, but so far I think this is
the better option.

> > While studying this I noticed a minor thinko in smgrrelease() in
> > 15+16, so here's a fix for that also.  I haven't figured out a
> > sequence that makes anything bad happen, but we should really
> > invalidate smgr_targblock when a relfilenode is reused, since it might
> > point past the end.  This becomes more obvious once smgrrelease() is
> > used for truncation, as proposed here.
>
> +1. You can move the smgr_targblock clearing out of the loop, though.

Right, thanks.  Pushed.