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
