Hi, Thank you for looking into this all!
On Tue, 22 Sept 2026 at 03:12, shihao zhong <[email protected]> wrote: > > Right, the per IO check puts the spinlock in the hot path. > > The barrier also only releases, it does not destroy, so it does not > fix what Nazir reported. The hash table in the IO worker keeps > growing either way. Nothing pins those entries in an IO worker, and > smgrdestroyall only zaps unpinned ones, so destroying them there > should be safe. Correct. > That suggests a version with no lock at all. Keep the cleanup where > Nazir put it, at a safe point in the worker loop, but trigger it on a > local condition, the number of unpinned entries being over a cap, > rather than on a checkpoint generation. No shared state, and it also > covers a worker that never goes idle. We need to make a function call to hash_get_num_entries() for each IO to check number of unpinned entries, which doesn't sound good to me. This is especially true for the partitioned case. For now, I used the number of IOs (capped at 1024) since the last smgr cleanup. This solution removes CheckpointerShmem->ckpt_lck contention and worker goes idle problem. -- Regards, Nazir Bilal Yavuz Microsoft
From 3c217dfdfbae703aa1c293b4aade10b8c9977a26 Mon Sep 17 00:00:00 2001 From: Nazir Bilal Yavuz <[email protected]> Date: Wed, 23 Sep 2026 14:33:18 +0300 Subject: [PATCH v2] aio: worker: Periodically free SMGR objects 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 after every 1024 completed IO requests, using a worker-local counter. Perform cleanup after IO completion and error-context clearing, when no borrowed descriptors or SMGR references remain in use. Discussion: https://postgr.es/m/CAN55FZ2BesKUnajdgpw1fPSe3S6_CHOugryaUEtD7vdP%3DdRKEQ%40mail.gmail.com --- src/backend/storage/aio/method_worker.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/backend/storage/aio/method_worker.c b/src/backend/storage/aio/method_worker.c index cf75b2816b7..a4fabde6e5b 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 completed IOs between SMGR cache cleanups. */ +#define PGAIO_WORKER_SMGR_CLEANUP_INTERVAL 1024 + /* Debugging support: show current IO and wakeups:ios statistics in ps. */ /* #define PGAIO_WORKER_SHOW_PS_INFO */ @@ -695,6 +699,7 @@ IoWorkerMain(const void *startup_data, size_t startup_data_len) char cmd[128]; int hist_ios = 0; int hist_wakeups = 0; + int ios_since_smgr_cleanup = 0; AuxiliaryProcessMainCommon(); @@ -954,6 +959,18 @@ 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. Periodically destroy them based on a local IO counter. + * The IO has completed and its error context has been cleared, so + * no borrowed file descriptors or SMGR references remain in use. + */ + if (++ios_since_smgr_cleanup >= PGAIO_WORKER_SMGR_CLEANUP_INTERVAL) + { + smgrdestroyall(); + ios_since_smgr_cleanup = 0; + } } else { -- 2.47.3
