Re: [patch] Cache invalidation for I/O Workers

On Sun, Sep 20, 2026 at 10:17 PM shihao zhong <zhong950419(at)gmail(dot)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.
https://www.postgresql.org/message-id/CAN55FZ2BesKUnajdgpw1fPSe3S6_CHOugryaUEtD7vdP=dRKEQ@mail.gmail.com

Regards,
Alexandre

>