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
