On 14/08/2023 05:41, Thomas Munro wrote:
The new idea is to overload smgrrelease(it) so that it also clears the
owner, which means that AtEOXact_SMgr() will eventually smgrclose(it),
unless it is re-owned by a relation before then. That choice stems
from the complete lack of information available via sinval in the case
of an overflow. We must (1) close all descriptors because any file
might have been unlinked, (2) keep all pointers valid and yet (3) not
leak dropped smgr objects forever. In this patch, smgrreleaseall()
achieves those goals.
Makes sense.
Some of the smgrclose() calls could perhaps still be smgrclose(). If you
can guarantee that no-one is holding the SMgrRelation, it's still OK to
call smgrclose(). There's little gain from closing earlier, though.
Proof-of-concept patch attached. Are there holes in this scheme?
Better ideas welcome. In terms of spelling, another option would be
to change the behaviour of smgrclose() to work as described, ie it
would call smgrrelease() and then also disown, so we don't have to
change most of those callers, and then add a new function
smgrdestroy() for the few places that truly need it. Or something
like that.
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.
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.
--
Heikki Linnakangas
Neon (https://neon.tech)
