[patch] Cache invalidation for I/O Workers

Hi Alexandre,

Thanks for looking into this issue. It seems the patch is missing from the
attachment.

Also, would you mind creating a new entry in commifeast?

Thanks,
Shihao

The forgotten patch

On Fri, Sep 18, 2026 at 5:59 PM shihao zhong <zhong950419@gmail.com> wrote:

Hi Alexandre,

Thanks for looking into this issue. It seems the patch is missing from the attachment.

Also, would you mind creating a new entry in commifeast?

https://commitfest.postgresql.org/patch/7322/

Hi Alexandre,

Thanks for the patch. I read v1, here are my suggestions.

+ if (msg->id == SHAREDINVALSMGR_ID)
+ {
+ RelFileLocatorBackend rlocator;
+ rlocator.locator = msg->sm.rlocator;
+ rlocator.backend = (msg->sm.backend_hi << 16) | (int) msg->sm.backend_lo;
+ smgrreleaserellocator(rlocator);
+ }

Since 21d9c3ee4ef, smgrreleaserellocator() only closes the fds. The
SMgrRelation
entry stays in the hash table. Backends free those in AtEOXact_SMgr(), but
an
IO worker never runs a transaction, so entries for dropped relations
pile up. The fd leak turns into a memory leak. We had runs into production
issue because of that.

bgwriter and checkpointer have the same problem. They call
smgrdestroyall() once per checkpoint cycle, see the comment in
bgwriter.c. Could the IO worker just do the same every so often? That
is option 1 in the bug report. It needs no sinval slot for up to 32
workers, and it is small enough to backpatch to 18, where
io_method=worker is the default.

The patch also needs a pgindent run.

Thanks,
Shihao

On Sun, Sep 20, 2026 at 10:17 PM shihao zhong <zhong950419@gmail.com> wrote:

Hi Alexandre,

Thanks for the patch. I read v1, here are my suggestions.

+ if (msg->id == SHAREDINVALSMGR_ID)
+ {
+ RelFileLocatorBackend rlocator;
+ rlocator.locator = msg->sm.rlocator;
+ rlocator.backend = (msg->sm.backend_hi << 16) | (int) msg->sm.backend_lo;
+ smgrreleaserellocator(rlocator);
+ }

Since 21d9c3ee4ef, smgrreleaserellocator() only closes the fds. The
SMgrRelation
entry stays in the hash table. Backends free those in AtEOXact_SMgr(), but
an
IO worker never runs a transaction, so entries for dropped relations
pile up. The fd leak turns into a memory leak. We had runs into production
issue because of that.

I see now, what I understood is that smgrrelease() only closes,
smgrdestroy() closes
and releases memory. Updated the patch and the function comments trying to
make this
distinction more evident.

bgwriter and checkpointer have the same problem. They call

smgrdestroyall() once per checkpoint cycle, see the comment in
bgwriter.c. Could the IO worker just do the same every so often?

I saw a proposal for what you described [1], but it doesn't seem ideal as
It keeps stale handles for much longer than necessary, and closes files
unnecessarily after each checkpoint.

That
is option 1 in the bug report. It needs no sinval slot for up to 32
workers,

Are the sinval entries concerning? If so, I think it could have something
way simpler,
since we have a reset for when the buffer overflows, all we would need to
have is
a circular queue of messages and each worker saving its position. If
pending messages
is above the circular queue capacity it is a reset. This way the
invalidation list doesn't
even need to be aware of who is reading it.

and it is small enough to backpatch to 18, where
io_method=worker is the default.

Indeed that is smaller, maybe backpatch with [1] and using this going
forward.

The patch also needs a pgindent run.

Applied this time.

While checking SharedInvalBackendInit I noticed this
sinval.c
287 /*
288 * This can run in parallel with read operations, but not with write
289 * operations, since SIInsertDataEntries relies on the pgprocnos array to
290 * set hasMessages appropriately.
291 */
292 LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE);

It sounds like we could use a LW_SHARED lock there.

1.
/messages/by-id/CAN55FZ2BesKUnajdgpw1fPSe3S6_CHOugryaUEtD7vdP=dRKEQ@mail.gmail.com

Regards,
Alexandre

Hi Alexandre,

v2 looks right to me, the entries are freed now.

An idle worker sleeps in WaitLatch, so it only sees the messages after
a catchup signal, that is 2048 queued messages. On a quiet system the
stale fds stay, like in the 51 minutes case from the bug report.
Nazir's patch covers that case, yours covers the busy worker. Maybe the
two threads should be merged, CCing Nazir.

LW_SHARED does not work there. SharedInvalBackendInit does
pgprocnos[numProcs++] = MyProcNumber, two backends starting at the same
time would both hold the shared lock and race on numProcs.

Small things: a catchup wakeup is counted in hist_wakeups, the comment
rewrites in smgr.c are unrelated, and shmem.h sorts before sinvaladt.h.

Thanks,
Shihao