Re: Optimize LISTEN/NOTIFY

From: "Joel Jacobson" <joel(at)compiler(dot)org>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Arseniy Mukhin" <arseniy(dot)mukhin(dot)dev(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Optimize LISTEN/NOTIFY
Date: 2025-10-16 09:39:02
Message-ID: 3ef9b541-e835-4dc8-8f7a-b76677f17b36@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 15, 2025, at 16:16, Tom Lane wrote:
> Arseniy Mukhin <arseniy(dot)mukhin(dot)dev(at)gmail(dot)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

Attachment Content-Type Size
0001-optimize_listen_notify-v19.patch application/octet-stream 9.3 KB
0002-optimize_listen_notify-v19.patch application/octet-stream 31.3 KB
0002-optimize_listen_notify-v19-alt1.txt text/plain 3.9 KB
0002-optimize_listen_notify-v19-alt3.txt text/plain 7.4 KB
0002-optimize_listen_notify-v19-alt2.txt text/plain 3.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Xuneng Zhou 2025-10-16 09:39:20 Re: pgstattuple: Use streaming read API in pgstatindex functions
Previous Message Mircea Cadariu 2025-10-16 09:27:25 Re: [BUG] temporary file usage report with extended protocol and unnamed portals