public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tom Lane <[email protected]>
To: Amit Langote <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: David Rowley <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: generic plans and "initial" pruning
Date: Thu, 19 Jan 2023 22:31:49 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+HiwqFhQ8tLMLQAWYRWiBQUrWSLM8qhVzJ4B5jWr=orUXfSyA@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CA+HiwqFhQ8tLMLQAWYRWiBQUrWSLM8qhVzJ4B5jWr=orUXfSyA@mail.gmail.com>
Amit Langote <[email protected]> writes:
> On Fri, Jan 20, 2023 at 4:39 AM Tom Lane <[email protected]> wrote:
>> I had what felt like an epiphany: the whole problem arises because the
>> system is wrongly factored. We should get rid of AcquireExecutorLocks
>> altogether, allowing the plancache to hand back a generic plan that
>> it's not certain of the validity of, and instead integrate the
>> responsibility for acquiring locks into executor startup.
> Interesting. The current implementation relies on
> PlanCacheRelCallback() marking a generic CachedPlan as invalid, so
> perhaps there will have to be some sharing of state between the
> plancache and the executor for this to work?
Yeah. Thinking a little harder, I think this would have to involve
passing a CachedPlan pointer to the executor, and what the executor
would do after acquiring each lock is to ask the plancache "hey, do
you still think this CachedPlan entry is valid?". In the case where
there's a problem, the AcceptInvalidationMessages call involved in
lock acquisition would lead to a cache inval that clears the validity
flag on the CachedPlan entry, and this would provide an inexpensive
way to check if that happened.
It might be possible to incorporate this pointer into PlannedStmt
instead of passing it separately.
>> * In a successfully built execution state tree, there will simply
>> not be any nodes corresponding to pruned-away, never-locked subplans.
> I think this is true with the patch as proposed too, but I was still a
> bit worried about what an ExecutorStart_hook may be doing with an
> uninitialized plan tree. Maybe we're mandating that the hook must
> call standard_ExecutorStart() and only work with the finished
> PlanState tree?
It would certainly be incumbent on any such hook to not touch
not-yet-locked parts of the plan tree. I'm not particularly concerned
about that sort of requirements change, because we'd be breaking APIs
all through this area in any case.
>> * In some cases (views, at least) we need to acquire lock on relations
>> that aren't directly reflected anywhere in the plan tree. So there'd
>> have to be a separate mechanism for getting those locks and rechecking
>> validity afterward. A list of relevant relation OIDs might be enough
>> for that.
> Hmm, a list of only the OIDs wouldn't preserve the lock mode,
Good point. I wonder if we could integrate this with the
RTEPermissionInfo data structure?
> Would you like me to hack up a PoC or are you already on that?
I'm not planning to work on this myself, I was hoping you would.
regards, tom lane
view thread (22+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: generic plans and "initial" pruning
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox