From 0928ad2dc3acf0165c562093bf35f2cc84ecd912 Mon Sep 17 00:00:00 2001 From: rahila Date: Thu, 10 Sep 2026 16:08:30 +0800 Subject: [PATCH] Add RESOURCE_RELEASE_AT_ONCE phase, remove ResourceOwnerReleaseAllOfKind plpgsql keeps three detached ResourceOwners that must survive internal COMMIT/ROLLBACK inside a procedure or DO block, and that only ever accumulate plan-cache refcounts. They used to be drained with the single-kind ResourceOwnerReleaseAllOfKind(), which bypasses the normal phased release machinery entirely: it scans the array and hash table directly and calls each resource's ReleaseResource callback without sorting, without tracking release_phase/priority, and without ever setting 'sorted'. This commit adds a new ResourceReleasePhase value, RESOURCE_RELEASE_AT_ONCE, that means exactly that. A single ResourceOwnerRelease(owner, RESOURCE_RELEASE_AT_ONCE, ...) call sorts the owner's resources as usual, then releases every remaining entry regardless of its individual release_phase, without breaking out of the loop between phases. Leak warnings are unconditionally suppressed for this phase, since by definition everything found is meant to be released here, not left over by mistake. The four plpgsql call sites now each make one such call instead of three phased ones. With ResourceOwnerReleaseAllOfKind() gone, 'releasing' and 'sorted' are always set together (sorting now happens at the same point 'releasing' is set the first time an owner is released), so the two booleans collapse into the single 'releasing' flag. plancache.c plancache.c's ReleaseAllPlanCacheRefsInOwner() wrapper, which existed only to call ResourceOwnerReleaseAllOfKind() and had no remaining callers, is removed along with it. --- src/backend/utils/cache/plancache.c | 9 --- src/backend/utils/resowner/resowner.c | 100 ++++++++------------------ src/include/utils/plancache.h | 2 - src/include/utils/resowner.h | 18 +++-- src/pl/plpgsql/src/pl_exec.c | 2 +- src/pl/plpgsql/src/pl_handler.c | 6 +- 6 files changed, 47 insertions(+), 90 deletions(-) diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index a1b406cee29..37bc54cbe99 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -2423,15 +2423,6 @@ ResetPlanCache(void) } } -/* - * Release all CachedPlans remembered by 'owner' - */ -void -ReleaseAllPlanCacheRefsInOwner(ResourceOwner owner) -{ - ResourceOwnerReleaseAllOfKind(owner, &planref_resowner_desc); -} - /* ResourceOwner callbacks */ static void diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index ac413d54837..83b23ce7266 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -118,13 +118,11 @@ struct ResourceOwnerData /* * When ResourceOwnerRelease is called, we sort the 'hash' and 'arr' by - * the release priority. After that, no new resources can be remembered - * or forgotten in retail. We have separate flags because - * ResourceOwnerReleaseAllOfKind() temporarily sets 'releasing' without - * sorting the arrays. + * the release priority, and set 'releasing'. After that, no new + * resources can be remembered or forgotten in retail, and 'hash'/'arr' + * are known to be sorted. */ bool releasing; - bool sorted; /* are 'hash' and 'arr' sorted by priority? */ /* * Number of items in the locks cache, array, and hash table respectively. @@ -354,7 +352,6 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase, * either in the array or the hash. */ Assert(owner->releasing); - Assert(owner->sorted); if (owner->nhash == 0) { items = owner->arr; @@ -374,6 +371,11 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase, * starting from the end, until we hit the end of the phase that we are * releasing now. We will continue from there when called again for the * next phase. + * + * RESOURCE_RELEASE_AT_ONCE means the caller wants everything the owner + * holds released right now, regardless of each resource's own + * release_phase, so we don't check phases or break out early in that + * case. */ while (nitems > 0) { @@ -381,9 +383,12 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase, Datum value = items[idx].item; const ResourceOwnerDesc *kind = items[idx].kind; - if (kind->release_phase > phase) - break; - Assert(kind->release_phase == phase); + if (phase != RESOURCE_RELEASE_AT_ONCE) + { + if (kind->release_phase > phase) + break; + Assert(kind->release_phase == phase); + } if (printLeakWarnings) { @@ -541,7 +546,6 @@ ResourceOwnerRemember(ResourceOwner owner, Datum value, const ResourceOwnerDesc * releasing. We already checked this in ResourceOwnerEnlarge. */ Assert(!owner->releasing); - Assert(!owner->sorted); if (owner->narr >= RESOWNER_ARRAY_SIZE) { @@ -577,7 +581,6 @@ ResourceOwnerForget(ResourceOwner owner, Datum value, const ResourceOwnerDesc *k */ if (owner->releasing) elog(ERROR, "ResourceOwnerForget called for %s after release started", kind->name); - Assert(!owner->sorted); /* Search through all items in the array first. */ for (int i = owner->narr - 1; i >= 0; i--) @@ -706,9 +709,10 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, */ if (!owner->releasing) { - Assert(phase == RESOURCE_RELEASE_BEFORE_LOCKS); - Assert(!owner->sorted); + Assert(phase == RESOURCE_RELEASE_BEFORE_LOCKS || + phase == RESOURCE_RELEASE_AT_ONCE); owner->releasing = true; + ResourceOwnerSort(owner); } else { @@ -719,11 +723,6 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, * ResourceOwner from AbortTransaction. */ } - if (!owner->sorted) - { - ResourceOwnerSort(owner); - owner->sorted = true; - } /* * Make CurrentResourceOwner point to me, so that the release callback @@ -805,6 +804,19 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, */ ResourceOwnerReleaseAll(owner, phase, isCommit); } + else if (phase == RESOURCE_RELEASE_AT_ONCE) + { + /* + * Release everything the owner holds right away, without regard to + * each resource's own release_phase. The owner is expected to hold + * only resource kinds it's fine to release outside the normal + * phase-locked sequence (e.g. no locks), and there's + * intentionally no leak warning: whatever is remembered here is + * exactly what this call means to release, not something left + * behind by mistake. + */ + ResourceOwnerReleaseAll(owner, phase, false); + } /* Let add-on modules get a chance too */ for (item = ResourceRelease_callbacks; item; item = next) @@ -817,57 +829,6 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, CurrentResourceOwner = save; } -/* - * ResourceOwnerReleaseAllOfKind - * Release all resources of a certain type held by this owner. - */ -void -ResourceOwnerReleaseAllOfKind(ResourceOwner owner, const ResourceOwnerDesc *kind) -{ - /* Mustn't call this after we have already started releasing resources. */ - if (owner->releasing) - elog(ERROR, "ResourceOwnerForget called for %s after release started", kind->name); - Assert(!owner->sorted); - - /* - * Temporarily set 'releasing', to prevent calls to ResourceOwnerRemember - * while we're scanning the owner. Enlarging the hash would cause us to - * lose track of the point we're scanning. - */ - owner->releasing = true; - - /* Array first */ - for (int i = 0; i < owner->narr; i++) - { - if (owner->arr[i].kind == kind) - { - Datum value = owner->arr[i].item; - - owner->arr[i] = owner->arr[owner->narr - 1]; - owner->narr--; - i--; - - kind->ReleaseResource(value); - } - } - - /* Then hash */ - for (uint32 i = 0; i < owner->capacity; i++) - { - if (owner->hash[i].kind == kind) - { - Datum value = owner->hash[i].item; - - owner->hash[i].item = (Datum) 0; - owner->hash[i].kind = NULL; - owner->nhash--; - - kind->ReleaseResource(value); - } - } - owner->releasing = false; -} - /* * ResourceOwnerDelete * Delete an owner object and its descendants. @@ -1040,7 +1001,6 @@ ReleaseAuxProcessResources(bool isCommit) isCommit, true); /* allow it to be reused */ AuxProcessResourceOwner->releasing = false; - AuxProcessResourceOwner->sorted = false; } /* diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h index a0355e79c28..260dcc82cf4 100644 --- a/src/include/utils/plancache.h +++ b/src/include/utils/plancache.h @@ -200,8 +200,6 @@ typedef struct CachedExpression extern void InitPlanCache(void); extern void ResetPlanCache(void); -extern void ReleaseAllPlanCacheRefsInOwner(ResourceOwner owner); - extern CachedPlanSource *CreateCachedPlan(const RawStmt *raw_parse_tree, const char *query_string, CommandTag commandTag); diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h index eb6033b4fdb..a61ae6ceda4 100644 --- a/src/include/utils/resowner.h +++ b/src/include/utils/resowner.h @@ -48,12 +48,22 @@ extern PGDLLIMPORT ResourceOwner AuxProcessResourceOwner; * resource types are given below, extensions may use any priority relative to * those or RELEASE_PRIO_FIRST/LAST. RELEASE_PRIO_FIRST is a fine choice if * your resource doesn't depend on any other resources. + * + * RESOURCE_RELEASE_AT_ONCE is different: it doesn't participate in the + * phased BEFORE_LOCKS/LOCKS/AFTER_LOCKS sequence at all. Example usage: for + * resource owners that are known in advance to hold only a single resource + * kind (so release ordering across kinds is moot) and that live outside the + * normal phase-locked release sequence, such as the detached ResourceOwners + * plpgsql uses to track plan-cache references across internal transactions. + * A single ResourceOwnerRelease() call with this phase releases everything + * the owner holds immediately, without waiting for further phase calls. */ typedef enum { RESOURCE_RELEASE_BEFORE_LOCKS = 1, RESOURCE_RELEASE_LOCKS, RESOURCE_RELEASE_AFTER_LOCKS, + RESOURCE_RELEASE_AT_ONCE, } ResourceReleasePhase; typedef uint32 ResourceReleasePriority; @@ -101,9 +111,9 @@ typedef struct ResourceOwnerDesc * * This is called for each resource in the resource owner, in the order * specified by 'release_phase' and 'release_priority' when the whole - * resource owner is been released or when ResourceOwnerReleaseAllOfKind() - * is called. The resource is implicitly removed from the owner, the - * callback function doesn't need to call ResourceOwnerForget. + * resource owner is been released. The resource is implicitly removed + * from the owner, the callback function doesn't need to call + * ResourceOwnerForget. */ void (*ReleaseResource) (Datum res); @@ -149,8 +159,6 @@ extern void ResourceOwnerEnlarge(ResourceOwner owner); extern void ResourceOwnerRemember(ResourceOwner owner, Datum value, const ResourceOwnerDesc *kind); extern void ResourceOwnerForget(ResourceOwner owner, Datum value, const ResourceOwnerDesc *kind); -extern void ResourceOwnerReleaseAllOfKind(ResourceOwner owner, const ResourceOwnerDesc *kind); - extern void RegisterResourceReleaseCallback(ResourceReleaseCallback callback, void *arg); extern void UnregisterResourceReleaseCallback(ResourceReleaseCallback callback, diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 341beb496b8..c6aa1bc3710 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -8838,7 +8838,7 @@ plpgsql_xact_cb(XactEvent event, void *arg) FreeExecutorState(shared_simple_eval_estate); shared_simple_eval_estate = NULL; if (shared_simple_eval_resowner) - ReleaseAllPlanCacheRefsInOwner(shared_simple_eval_resowner); + ResourceOwnerRelease(shared_simple_eval_resowner, RESOURCE_RELEASE_AT_ONCE, false, true); shared_simple_eval_resowner = NULL; } else if (event == XACT_EVENT_ABORT || diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c index 3055c3db5d2..3d4a827ff47 100644 --- a/src/pl/plpgsql/src/pl_handler.c +++ b/src/pl/plpgsql/src/pl_handler.c @@ -289,7 +289,7 @@ plpgsql_call_handler(PG_FUNCTION_ARGS) /* Be sure to release the procedure resowner if any */ if (procedure_resowner) { - ReleaseAllPlanCacheRefsInOwner(procedure_resowner); + ResourceOwnerRelease(procedure_resowner, RESOURCE_RELEASE_AT_ONCE, false, true); ResourceOwnerDelete(procedure_resowner); } } @@ -393,7 +393,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS) /* Clean up the private EState and resowner */ FreeExecutorState(simple_eval_estate); - ReleaseAllPlanCacheRefsInOwner(simple_eval_resowner); + ResourceOwnerRelease(simple_eval_resowner, RESOURCE_RELEASE_AT_ONCE, false, true); ResourceOwnerDelete(simple_eval_resowner); /* Function should now have no remaining use-counts ... */ @@ -410,7 +410,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS) /* Clean up the private EState and resowner */ FreeExecutorState(simple_eval_estate); - ReleaseAllPlanCacheRefsInOwner(simple_eval_resowner); + ResourceOwnerRelease(simple_eval_resowner, RESOURCE_RELEASE_AT_ONCE, false, true); ResourceOwnerDelete(simple_eval_resowner); /* Function should now have no remaining use-counts ... */ -- 2.39.5 (Apple Git-154)