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
