GetCachedPlan() refactor: move execution lock acquisition out

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: GetCachedPlan() refactor: move execution lock acquisition out
Date: 2026-04-14 08:19:44
Message-ID: CA+HiwqE1ntHy2h9zJ9v3MwAkoGAveSERcHWkDTTZnP0kxWqbKQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Over in the "generic plans and initial pruning" thread [1], I came to
think that the cleanest way to address the architectural issue behind
that thread is to make GetCachedPlan() do less execution-related work.
Specifically, it should stop acquiring execution locks itself, and
leave that decision to callers.

GetCachedPlan() has long combined plan acquisition with execution
setup, and that coupling makes it harder to improve either side
cleanly. Tom pointed in this direction back in 2023 [2]:

"...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."

This patch takes the first half of that suggestion by moving execution
locking out of the plancache and into its callers.

The attached patch does that, with no intended behavioral change:

* Remove AcquireExecutorLocks() from CheckCachedPlan(), so
GetCachedPlan() returns a plan without holding execution locks.

* Add a new internal helper in plancache.c, LockCachedPlan(), which
acquires execution locks, rechecks validity, and returns false if the
plan was invalidated, so the caller can retry.

* Add GetCachedPlanLocked() as a convenience wrapper that fetches a
plan, calls LockCachedPlan(), and retries on invalidation. This
becomes the normal entry point for callers that want a plan ready for
execution.

* Convert existing GetCachedPlan() callers to use GetCachedPlanLocked().

The second half of Tom's suggestion, moving that responsibility into
executor startup, is trickier. ExecutorStart() is a stable API with
extension hooks, so changing its contract has a fairly high bar. I
tried that route once already [3].

A working variant that adds an ExecutorPrep() entry point, with a
wrapper implementing pruning-aware locking on top, was discussed in
the original thread [1]. But rather than propose that whole stack at
once, I think it is better to do this in phases: first decouple plan
acquisition from execution locking, then revisit the ExecutorPrep()
piece separately if this goes in.

The eventual goal is still pruning-aware locking. I'm posting this
piece separately because the original thread has become fairly long,
and this part seems easier to review on its own.

[1] https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
[2] https://postgr.es/m/4191508.1674157166%40sss.pgh.pa.us
[3] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=1722d5eb05d8e5d2e064cd1798abcae4f296ca9d

--
Thanks, Amit Langote

Attachment Content-Type Size
v1-0001-Move-execution-lock-acquisition-out-of-GetCachedP.patch application/octet-stream 10.7 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2026-04-14 08:34:55 Re: Support EXCEPT for TABLES IN SCHEMA publications
Previous Message Peter Smith 2026-04-14 08:09:42 Re: Support EXCEPT for TABLES IN SCHEMA publications