Re: aio: worker: Free SMGR objects when idle

On Mon, Sep 21, 2026 at 4:09 PM Andres Freund <andres(at)anarazel(dot)de> wrote:

I'm doubtful that that'd be a good idea. It'd add a lot of contention on the
> CheckpointerShmem->ckpt_lck that's acquired as part of
> FirstCallSinceLastCheckpoint. On a system with a lot of IO that'd probably
> noticeable.
>

I always look forward to your comments Andres.
I don't see a reason to have a lock in FirstCallSinceLastCheckpoint()

diff --git a/src/backend/postmaster/checkpointer.c
b/src/backend/postmaster/checkpointer.c
index 580c7944119..c91c0350e78 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -1517,9 +1517,7 @@ FirstCallSinceLastCheckpoint(void)
int new_done;
bool FirstCall = false;

- SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
new_done = CheckpointerShmem->ckpt_done;
- SpinLockRelease(&CheckpointerShmem->ckpt_lck);

if (new_done != ckpt_done)
FirstCall = true;

> I don't think any approach that adds an acuisition of a central lock around
> every IO is going to make sense here.
>
> I think Alexandre's patch has the same issue, unfortunately.
>

Unfortunately, yes. I didn't notice how complex ReceiveSharedInvalidMessages
is, SIGetDataEntries is hiding some LWLocks.

I'm sure we could make FirstCallSinceLastCheckpoint() not require a
> spinlock.

race condition hehe

> But I wonder if that's quite the right design. Doing work for every
> single iteration of various loops (bgwriter, io worker) doesn't really seem
> right to me for something that's as rare as this.

Well, from my previous read of Nazir's patch he will check only when the
worker
goes idle, at most once per checkpoint.

> Having aux processes participate in sinval doesn't really clearly seem like
> the right thing either. There are a lot of messages that they never benefit
> from. And manual filtering in the receive function like Alexandre added
> doesn't really seem like a good way of addressing that.
>

Noted

> Maybe we should just drop having SHAREDINVALSMGR_ID and only use
> PROCSIGNAL_BARRIER_SMGRRELEASE?
>

Closing all the files because one was dropped doesn't seem right either.

bool
ProcessBarrierSmgrRelease(void)
{
smgrreleaseall();
return true;
}

And, apparently, that is prone to the same issue as shihao pointed out
earlier.
smgrreleaseall() will turn a file leak into a slow memory memory leak.

Regards,
Alexandre