Re: Optimize LISTEN/NOTIFY
Joel Jacobson <joel@compiler.org>
From: "Joel Jacobson" <joel@compiler.org>
To: "Tom Lane" <tgl@sss.pgh.pa.us>,
"Arseniy Mukhin" <arseniy.mukhin.dev@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-10-16T09:39:02Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Optimize LISTEN/NOTIFY via shared channel map and direct advancement.
- 282b1cde9ded 19 (unreleased) landed
-
Fix incorrect logic for caching ResultRelInfos for triggers
- 39dcfda2d23a 19 (unreleased) cited
Attachments
- 0001-optimize_listen_notify-v19.patch (application/octet-stream) patch v19-0001
- 0002-optimize_listen_notify-v19.patch (application/octet-stream) patch v19-0002
- 0002-optimize_listen_notify-v19-alt1.txt (text/plain)
- 0002-optimize_listen_notify-v19-alt3.txt (text/plain)
- 0002-optimize_listen_notify-v19-alt2.txt (text/plain)
On Wed, Oct 15, 2025, at 16:16, Tom Lane wrote: > Arseniy Mukhin <arseniy.mukhin.dev@gmail.com> writes: >> I think "Direct advancement" is a good idea. But the way it's >> implemented now has a concurrency bug. Listeners store its current >> position in the local variable 'pos' during the reading in >> asyncQueueReadAllNotifications() and don't hold NotifyQueueLock. It >> means that some notifier can directly advance the listener's position >> while the listener has an old value in the local variable. The same >> time we use listener positions to find out the limit we can truncate >> the queue in asyncQueueAdvanceTail(). > > Good catch! I've implemented the three ideas presented below, attached as .txt files that are diffs on top of v19, which has these changes since v17: 0002-optimize_listen_notify-v19.patch: * Improve wording of top comment per request from Chao Li. * Add initChannelHash call to top of SignalBackends, to fix bug reported by Arseniy Mukhin. > I think we can perhaps salvage the idea if we invent a separate > "advisory" queue position field, which tells its backend "hey, > you could skip as far as here if you want", but is not used for > purposes of SLRU truncation. Above idea is implemented in 0002-optimize_listen_notify-v19-alt1.txt > Alternatively, split the queue pos > into "this is where to read next" and "this is as much as I'm > definitively done with", where the second field gets advanced at > the end of asyncQueueReadAllNotifications. Not sure which > view would be less confusing (in the end I guess they're nearly > the same thing, differently explained). Above idea is implemented in 0002-optimize_listen_notify-v19-alt2.txt > A different line of thought could be to get rid of > asyncQueueReadAllNotifications's optimization of moving the > queue pos only once, per > > * (We could alternatively retake NotifyQueueLock and move the position > * before handling each individual message, but that seems like too much > * lock traffic.) > > Since we only need shared lock to advance our own queue pos, > maybe that wouldn't be too awful. Not sure. Above idea is implemented in 0002-optimize_listen_notify-v19-alt3.txt /Joel