From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Jingtang Zhang Date: Tue, 03 Nov 2026 21:59:41 +0800 Subject: [PATCH v3 alternative A] Make resource owners reusable Provide a public operation that restores an already released resource owner and its descendants for reuse. Keep the owner objects and their allocated storage, and use the operation for auxiliary-process cleanup. Sorting and bulk release can leave stale hash entries behind. Reusing an owner without clearing them can release old resources again or loop while searching for a free hash slot. Clear moved source slots during compaction and clear retired hash entries before calling their release callbacks. This leaves the hash storage ready for reuse without a full-table clearing pass during reset. --- src/backend/utils/resowner/README | 7 ++ src/backend/utils/resowner/resowner.c | 50 +++++++++++-- src/include/utils/resowner.h | 1 + .../test_resowner/expected/test_resowner.out | 27 +++++++ .../modules/test_resowner/sql/test_resowner.sql | 8 ++ .../modules/test_resowner/test_resowner--1.0.sql | 4 + .../modules/test_resowner/test_resowner_basic.c | 87 ++++++++++++++++++++++ 7 files changed, 176 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/resowner/README b/src/backend/utils/resowner/README index cbf34e0b5646a3f1675d9c91cdabe769e7a73278..9b0035c97d46e7d78dea2534d7ad2678a72bafec 100644 --- a/src/backend/utils/resowner/README +++ b/src/backend/utils/resowner/README @@ -51,6 +51,9 @@ The basic operations on a ResourceOwner are: * release a ResourceOwner's assets (free all owned resources, but not the owner object itself) +* reset a ResourceOwner (including child owner objects) for reuse; all resources + must have been released beforehand + * delete a ResourceOwner (including child owner objects); all resources must have been released beforehand @@ -169,6 +172,10 @@ the ResourceOwner to remember any more resources. You also cannot call ResourceOwnerForget on the resource owner to release any previously remembered resources "in retail", after you have started the release process. +After all three release phases have completed, ResourceOwnerReset() can be +used to make the ResourceOwner and its descendants reusable. The owner +objects and their allocated storage are retained. + Normally, you are expected to call ResourceOwnerForget on every resource so that at commit, the ResourceOwner is empty (locks are an exception). If there are any resources still held at commit, ResourceOwnerRelease will print a diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index ac413d54837384455cc68ddc6a3b68d2db17db50..38a1d52ea9b41551ff46401d384bb838ccb83b03 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -148,8 +148,9 @@ struct ResourceOwnerData * too much time searching for empty slots. * * If 'sorted' is set, the contents are no longer hashed, but sorted by - * release priority. The first 'nhash' elements are occupied, the rest - * are empty. + * release priority. The first 'nhash' elements contain the remaining + * resources; entries after that are not used until the owner is reset or + * deleted. */ ResourceElem *hash; uint32 capacity; /* allocated length of hash[] */ @@ -309,7 +310,11 @@ ResourceOwnerSort(ResourceOwner owner) if (owner->hash[idx].kind != NULL) { if (dst != idx) + { owner->hash[dst] = owner->hash[idx]; + owner->hash[idx].item = 0; + owner->hash[idx].kind = NULL; + } dst++; } } @@ -397,15 +402,19 @@ ResourceOwnerReleaseAll(ResourceOwner owner, ResourceReleasePhase phase, } /* - * Update stored count to forget the item before calling its - * ReleaseResource method. This avoids double-free crashes in case an - * error gets thrown within ReleaseResource. + * Update stored count and clear the hash slot to forget the item + * before calling its ReleaseResource method. This avoids double-free + * crashes in case an error gets thrown within ReleaseResource. */ nitems--; if (using_arr) owner->narr = nitems; else + { owner->nhash = nitems; + items[idx].item = 0; + items[idx].kind = NULL; + } kind->ReleaseResource(value); } @@ -659,7 +668,8 @@ ResourceOwnerForget(ResourceOwner owner, Datum value, const ResourceOwnerDesc *k * resources can be remembered in the resource owner. You also cannot call * ResourceOwnerForget on any previously remembered resources to release * resources "in retail" after that, you must let the bulk release take care - * of them. + * of them. Once all three release phases have completed, ResourceOwnerReset + * can be used to make the owner reusable. */ void ResourceOwnerRelease(ResourceOwner owner, @@ -868,6 +878,31 @@ ResourceOwnerReleaseAllOfKind(ResourceOwner owner, const ResourceOwnerDesc *kind owner->releasing = false; } +/* + * ResourceOwnerReset + * Reset an owner object and its descendants for reuse. + * + * The caller must have completed all three release phases for the object tree. + * The owner objects and their allocated storage are retained. + */ +void +ResourceOwnerReset(ResourceOwner owner) +{ + ResourceOwner child; + + Assert(owner->narr == 0); + Assert(owner->nhash == 0); + Assert(owner->nlocks == 0 || owner->nlocks == MAX_RESOWNER_LOCKS + 1); + Assert(dlist_is_empty(&owner->aio_handles)); + + for (child = owner->firstchild; child != NULL; child = child->nextchild) + ResourceOwnerReset(child); + + owner->nlocks = 0; + owner->releasing = false; + owner->sorted = false; +} + /* * ResourceOwnerDelete * Delete an owner object and its descendants. @@ -1039,8 +1074,7 @@ ReleaseAuxProcessResources(bool isCommit) RESOURCE_RELEASE_AFTER_LOCKS, isCommit, true); /* allow it to be reused */ - AuxProcessResourceOwner->releasing = false; - AuxProcessResourceOwner->sorted = false; + ResourceOwnerReset(AuxProcessResourceOwner); } /* diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h index eb6033b4fdb6531d188fe145bb9b5267530885d8..65c73628bb181efe993ac6bd77ec32847119eb3d 100644 --- a/src/include/utils/resowner.h +++ b/src/include/utils/resowner.h @@ -140,6 +140,7 @@ extern void ResourceOwnerRelease(ResourceOwner owner, ResourceReleasePhase phase, bool isCommit, bool isTopLevel); +extern void ResourceOwnerReset(ResourceOwner owner); extern void ResourceOwnerDelete(ResourceOwner owner); extern ResourceOwner ResourceOwnerGetParent(ResourceOwner owner); extern void ResourceOwnerNewParent(ResourceOwner owner, diff --git a/src/test/modules/test_resowner/expected/test_resowner.out b/src/test/modules/test_resowner/expected/test_resowner.out index 527b678fc12802dd429cb5c9284a0063324f202e..28df6e7b141f90c84b91ebc84558f9168847cd10 100644 --- a/src/test/modules/test_resowner/expected/test_resowner.out +++ b/src/test/modules/test_resowner/expected/test_resowner.out @@ -195,3 +195,30 @@ ERROR: ResourceOwnerEnlarge called after release started SELECT test_resowner_forget_between_phases(); ERROR: ResourceOwnerForget called for test resource after release started reset client_min_messages; +-- Reuse an auxiliary resource owner with and without a hash table. +-- For hash-table cases, force a resize before inserting into the reused hash. +-- This exposes stale entries without hanging while looking for a free slot. +SELECT test_resowner_aux(16); + test_resowner_aux +------------------- + +(1 row) + +SELECT test_resowner_aux(33); + test_resowner_aux +------------------- + +(1 row) + +SELECT test_resowner_aux(48); + test_resowner_aux +------------------- + +(1 row) + +SELECT test_resowner_aux(64); + test_resowner_aux +------------------- + +(1 row) + diff --git a/src/test/modules/test_resowner/sql/test_resowner.sql b/src/test/modules/test_resowner/sql/test_resowner.sql index 23284b7c8b91cf2e635f45cd495b3e5e4be1d04d..d42b69f33dbc1a352478649300767f7622b0ecbe 100644 --- a/src/test/modules/test_resowner/sql/test_resowner.sql +++ b/src/test/modules/test_resowner/sql/test_resowner.sql @@ -23,3 +23,11 @@ set client_min_messages='warning'; -- order between ERROR and NOTICE varies SELECT test_resowner_remember_between_phases(); SELECT test_resowner_forget_between_phases(); reset client_min_messages; + +-- Reuse an auxiliary resource owner with and without a hash table. +-- For hash-table cases, force a resize before inserting into the reused hash. +-- This exposes stale entries without hanging while looking for a free slot. +SELECT test_resowner_aux(16); +SELECT test_resowner_aux(33); +SELECT test_resowner_aux(48); +SELECT test_resowner_aux(64); diff --git a/src/test/modules/test_resowner/test_resowner--1.0.sql b/src/test/modules/test_resowner/test_resowner--1.0.sql index 26fed7a010ca86ab800fd01c88ef4d84a89b6866..48dc6ed5d2c8e6313877c125cd7ecd26c35e878d 100644 --- a/src/test/modules/test_resowner/test_resowner--1.0.sql +++ b/src/test/modules/test_resowner/test_resowner--1.0.sql @@ -19,6 +19,10 @@ CREATE FUNCTION test_resowner_forget_between_phases() RETURNS pg_catalog.void AS 'MODULE_PATHNAME' LANGUAGE C; +CREATE FUNCTION test_resowner_aux(nresources pg_catalog.int4) + RETURNS pg_catalog.void + AS 'MODULE_PATHNAME' LANGUAGE C; + CREATE FUNCTION test_resowner_many( nkinds pg_catalog.int4, nremember_bl pg_catalog.int4, diff --git a/src/test/modules/test_resowner/test_resowner_basic.c b/src/test/modules/test_resowner/test_resowner_basic.c index c0891f7e539830cf0140b6fdb1226e0cd57cf882..fca9fb246b7b9760187ab983018163e9d445f9c5 100644 --- a/src/test/modules/test_resowner/test_resowner_basic.c +++ b/src/test/modules/test_resowner/test_resowner_basic.c @@ -19,6 +19,7 @@ PG_MODULE_MAGIC; static void ReleaseString(Datum res); static char *PrintString(Datum res); +static void CountRelease(Datum res); /* * A resource that tracks strings and prints the string when it's released. @@ -207,3 +208,89 @@ test_resowner_forget_between_phases(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + +static void +CountRelease(Datum res) +{ + int *nreleased = (int *) DatumGetPointer(res); + + (*nreleased)++; +} + +/* + * Check that an auxiliary resource owner can be reused after bulk release. + */ +PG_FUNCTION_INFO_V1(test_resowner_aux); +Datum +test_resowner_aux(PG_FUNCTION_ARGS) +{ + static const ResourceOwnerDesc before_desc = { + .name = "test resource before locks", + .release_phase = RESOURCE_RELEASE_BEFORE_LOCKS, + .release_priority = RELEASE_PRIO_FIRST, + .ReleaseResource = CountRelease + }; + static const ResourceOwnerDesc after_desc = { + .name = "test resource after locks", + .release_phase = RESOURCE_RELEASE_AFTER_LOCKS, + .release_priority = RELEASE_PRIO_FIRST, + .ReleaseResource = CountRelease + }; + int32 nresources = PG_GETARG_INT32(0); + int *nreleased[2]; + ResourceOwner resowner; + ResourceOwner save_current = CurrentResourceOwner; + ResourceOwner save_aux = AuxProcessResourceOwner; + + if (nresources <= 0) + elog(ERROR, "nresources must be greater than zero"); + + nreleased[0] = palloc0_array(int, nresources); + nreleased[1] = palloc0_array(int, nresources); + resowner = ResourceOwnerCreate(CurrentResourceOwner, "test auxiliary owner"); + + /* + * Use an isolated owner without registering an auxiliary-process exit + * hook. + */ + PG_TRY(); + { + CurrentResourceOwner = AuxProcessResourceOwner = resowner; + for (int round = 0; round < 2; round++) + { + for (int i = 0; i < nresources; i++) + { + const ResourceOwnerDesc *kind = i % 2 == 0 ? &before_desc : &after_desc; + + ResourceOwnerEnlarge(resowner); + ResourceOwnerRemember(resowner, + PointerGetDatum(&nreleased[round][i]), kind); + } + ReleaseAuxProcessResources(false); + } + } + PG_FINALLY(); + { + CurrentResourceOwner = save_current; + AuxProcessResourceOwner = save_aux; + } + PG_END_TRY(); + + ResourceOwnerDelete(resowner); + + /* + * Keep both rounds' counters alive to detect releases of stale entries. + */ + for (int round = 0; round < 2; round++) + { + for (int i = 0; i < nresources; i++) + { + if (nreleased[round][i] != 1) + elog(ERROR, "resource %d in round %d was released %d times", + i, round, nreleased[round][i]); + } + pfree(nreleased[round]); + } + + PG_RETURN_VOID(); +} base-commit: 2af6dd3370677d8b06cf2a9eb62f69ba11e12cab