| From: | "Joel Jacobson" <joel(at)compiler(dot)org> |
|---|---|
| To: | "Chao Li" <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Optimize LISTEN/NOTIFY |
| Date: | 2025-11-19 03:14:44 |
| Message-ID: | ce0f1d67-8265-4e7d-834b-807de5b1b03b@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2025-11-19 03:15:54 | Re: PRI?64 vs Visual Studio (2022) |
| Previous Message | Chao Li | 2025-11-19 03:13:00 | Re: [PATCH] Add pg_get_tablespace_ddl() function to reconstruct CREATE TABLESPACE statement |