Hi, On Wed, 23 Sept 2026 at 15:18, shihao zhong <[email protected]> wrote: > > On the entry count. Nothing is pinned in an IO worker, only the > relcache calls smgrpin(), which also answers Alexandre's question. So > the unpinned count is just hash_get_num_entries(), and that only reads > a counter. SMgrRelationHash is local and not partitioned. The IO > counter is fine with me too though.
You are right that SMgrRelationHash is local and not partitioned, I missed that. Then, I think using the 'number of unpinned entries' makes more sense because the IO count won't work well when multiple IOs use the same relation. I introduced the smgrnumentries() function in v3 to get the number of cached SMgrRelation objects. > One gap in v2. v1 cleaned up when the worker went idle, v2 does not. > A worker that does fewer than 1024 IOs and then sleeps keeps those > entries and their descriptors until it gets busy again. #19622 has > that case, workers idle for 51 minutes still holding descriptors of > dropped tables. Maybe also clean up in the idle branch when > ios_since_smgr_cleanup > 0. That needs no lock either. Done. One thing remains: if the cache doesn't grow enough and the worker doesn't go idle (i.e. all IOs process less than PGAIO_WORKER_SMGR_CLEANUP_THRESHOLD relation), we don't clear the SMGR objects but I think this is a correct behavior. -- Regards, Nazir Bilal Yavuz Microsoft
From 01503010099e9b415bc140229e3d6c58dd39fe87 Mon Sep 17 00:00:00 2001 From: Nazir Bilal Yavuz <[email protected]> Date: Thu, 24 Sep 2026 11:53:00 +0300 Subject: [PATCH v3] aio: worker: Bound SMGR cache growth IO workers create SMGR objects when reopening relations, but don't have a transaction-end cleanup to destroy them. Long-lived workers can therefore retain entries for an increasing number of relations, including dropped ones. Destroy these objects when the worker's SMGR hash reaches 1024 entries, and release any remaining objects before sleeping. Workers do not pin SMGR objects, so the hash entry count is also the unpinned entry count. Discussion: https://postgr.es/m/CAN55FZ2BesKUnajdgpw1fPSe3S6_CHOugryaUEtD7vdP%3DdRKEQ%40mail.gmail.com --- src/backend/storage/aio/method_worker.c | 22 ++++++++++++++++++++++ src/backend/storage/smgr/smgr.c | 13 +++++++++++++ src/include/storage/smgr.h | 1 + 3 files changed, 36 insertions(+) diff --git a/src/backend/storage/aio/method_worker.c b/src/backend/storage/aio/method_worker.c index cf75b2816b7..a2271393179 100644 --- a/src/backend/storage/aio/method_worker.c +++ b/src/backend/storage/aio/method_worker.c @@ -45,6 +45,7 @@ #include "storage/pmsignal.h" #include "storage/proc.h" #include "storage/shmem.h" +#include "storage/smgr.h" #include "tcop/tcopprot.h" #include "utils/injection_point.h" #include "utils/memdebug.h" @@ -65,6 +66,9 @@ */ #define PGAIO_WORKER_WAKEUP_RATIO_SATURATE 4 +/* Number of SMGR entries that triggers cleanup while busy. */ +#define PGAIO_WORKER_SMGR_CLEANUP_THRESHOLD 1024 + /* Debugging support: show current IO and wakeups:ios statistics in ps. */ /* #define PGAIO_WORKER_SHOW_PS_INFO */ @@ -954,6 +958,17 @@ IoWorkerMain(const void *startup_data, size_t startup_data_len) RESUME_INTERRUPTS(); errcallback.arg = NULL; + + /* + * IO workers don't have transaction-end cleanup to destroy SMGR + * objects. Destroy them when the cache grows large enough. + * Workers don't pin SMGR objects, so all entries can be + * destroyed. The IO has completed and its error context has been + * cleared, so no borrowed file descriptors or SMGR references + * remain in use. + */ + if (smgrnumentries() >= PGAIO_WORKER_SMGR_CLEANUP_THRESHOLD) + smgrdestroyall(); } else { @@ -962,6 +977,13 @@ IoWorkerMain(const void *startup_data, size_t startup_data_len) /* Cancel new worker request if pending. */ pgaio_worker_cancel_grow(); + /* + * Release any remaining SMGR objects before sleeping. See + * PGAIO_WORKER_SMGR_CLEANUP_THRESHOLD for more information. + */ + if (smgrnumentries() > 0) + smgrdestroyall(); + /* Compute the remaining allowed idle time. */ if (io_worker_idle_timeout == -1) { diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index 5391640d861..acaa8b4ade6 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -288,6 +288,19 @@ smgropen(RelFileLocator rlocator, ProcNumber backend) return reln; } +/* + * smgrnumentries() -- Return the number of cached SMgrRelation objects, + * including pinned objects. + */ +int64 +smgrnumentries(void) +{ + if (SMgrRelationHash == NULL) + return 0; + + return hash_get_num_entries(SMgrRelationHash); +} + /* * smgrpin() -- Prevent an SMgrRelation object from being destroyed at end of * transaction diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index 09bd42fcf4b..16cfaeebcb3 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -112,6 +112,7 @@ extern void smgrwriteback(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, BlockNumber nblocks); extern BlockNumber smgrnblocks(SMgrRelation reln, ForkNumber forknum); extern BlockNumber smgrnblocks_cached(SMgrRelation reln, ForkNumber forknum); +extern int64 smgrnumentries(void); extern void smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, BlockNumber *old_nblocks, BlockNumber *nblocks); -- 2.47.3
