Thread

Re: Optimize LISTEN/NOTIFY

Joel Jacobson <joel@compiler.org> — 2025-11-19T03:14:44Z

On Tue, Nov 18, 2025, at 09:15, Chao Li wrote:
> Thanks for the continuous effort on this patch. Finally, I got some 
> time, after revisiting v28 throughoutly, I think it’s much better now. 

Thanks for reviewing.

> Just got 2 more comments:
>
...
> DSA is created and pinned by the first backend and every backend 
> isa_in_mapping, but I don’t see any unpin, is it a problem? If unpin is 
> not needed, why are they provided?

No, this is not a problem.

The channel hash area is pinned "so that it will continue to exist even
if all backends detach from it", via dsa_pin(). Each backend's mapping
lives for session lifetime via dsa_pin_mapping(). We never need to unpin
either. This follows the same pattern as e.g.
logicalrep_launcher_attach_dshmem() in launcher.c.

dsm_unpin_mapping() was added in f7102b0 (2014), but I cannot find any
use of it in the sources, I think it's there mostly for API
completeness.

> SignalBackends() now holds the dshash entry lock for long time, while 
> other backend’s LISTEN/UNLISTEN all needs to acquire the lock. So, my 
> suggestion is to copy the listeners array to local then quickly release 
> the lock.

Trying to optimize this further would mean increased code complexity,
since we would then have to worry and reason about stale reads.

I only think we should consider this if we find this to actually be a
bottleneck with the design, and my guess is that it's usually not
because:

1. dshash_find(..., false) in SignalBackends takes a shared lock, so
multiple concurrent SignalBackends() calls can read simultaneously.

2. The loop in SignalBackends is already I/O free, the region where we
do dshash_find(..., false) is within the same region that we hold the
exclusive lock; we're doing the expensive signaling after all locks have
been released.

3. We're already looping over numListeners while holding exclusive lock
on the channel in both Exec_ListenCommit and Exec_UnlistenCommit, so
what we're doing in SignalBackends isn't any worse.

4. We're not locking the entire channel hash, only the partition for one
channel at a time.

Just to be sure, I will do some LISTEN/UNLISTEN benchmarking to
investigate how the locking affects performance, and then we can
evaluate.

/Joel