Re: Optimize LISTEN/NOTIFY

On Sat, Nov 1, 2025, at 21:41, Arseniy Mukhin wrote:
> Thank you for working on this! There are few points about 'direct
> advancement' part:

Thanks for reviewing!

> Looks like the bug with truncating of the queue is gone, advancingPos
> does the trick, great.
>
> Maybe I missed something, but I failed to find an example where we can
> take advantage of advisoryPos:
>
> SignalBackends(void)
> ...
>
> if (QUEUE_POS_EQUAL(pos, queueHeadBeforeWrite))
> {
> ...
> if (!QUEUE_BACKEND_ADVANCING_POS(i))
> QUEUE_BACKEND_POS(i) = queueHeadAfterWrite;
> else if (QUEUE_POS_PRECEDES(advisoryPos, queueHeadAfterWrite))
> QUEUE_BACKEND_ADVISORY_POS(i) = queueHeadAfterWrite;
> }
>
> We update advisoryPos if:
> 1) listener's advancingPos is true
> 2) listener's pos equals queueHeadBeforeWrite
>
> (1) means the listener is currently reading. (2) means notifications
> that the listener is currently reading belong to us (or it's even
> possible that the listener is reading notifications that were added in
> the queue after ours). And since the listener is reading, it will only
> see updated advancingPos in the PG_FINALLY, where listener's pos will
> already be >= queueHeadAfterWrite (as result of reading).
>
>
> This condition seems to be redundant. I would say it should always be
> true, otherwise it would mean that somebody allowed the listener to
> skip our notification.
>
> else if (QUEUE_POS_PRECEDES(advisoryPos, queueHeadAfterWrite))

Ohhh, right! I agree with your reasoning; it's dead code.
This means we can remove the advisoryPos altogether, with
the benefit of making the code even simpler. That's what I've
done in v22, among some other changes.

Changes since v22:

* Optimize listening on thousands of channels per backend by replacing
the listenChannels List with a local hash table, renamed to
listenChannelsHash to avoid confusion.

* Removed advisoryPos, since it was not actually used. We only needed
advancingPos to fix the bug with truncation of the queue. It's possible
that the bottleneck in some workloads is no longer the wakeups, but I'm
not sure yet; I'll do some more benchmarking to get a better
understanding of whether it would be worthwhile to pursue further
optimization.

* Removed asyncQueuePageDiff, since it's no longer used.

Benchmark to demonstrate the effect of the listenChannelsHash:

% gcc -Wall -Wextra -O2 -pthread -I/Users/joel/pg19/include/postgresql/server -I/Users/joel/pg19/include -o async-notify-test-4 async-notify-test-4.c -L/Users/joel/pg19/lib -lpq -pthread -lm

v21:
% ./async-notify-test-4 --listeners 1 --notifiers 1 --channels 1 --extra-channels=10000
10 s: 1286593 sent (130036/s), 437822 received (44121/s)
0.00-0.01ms 0 (0.0%) avg: 0.000ms
0.01-0.10ms # 1 (0.0%) avg: 0.099ms
0.10-1.00ms # 39 (0.0%) avg: 0.576ms
1.00-10.00ms # 395 (0.1%) avg: 6.005ms
10.00-100.00ms # 6186 (1.4%) avg: 52.880mss
>100.00ms ######### 431214 (98.5%) avg: 3379.928ms

v22:
% ./async-notify-test-4 --listeners 1 --notifiers 1 --channels 1 --extra-channels=10000
10 s: 879208 sent (87704/s), 879207 received (87703/s)
0.00-0.01ms # 31 (0.0%) avg: 0.009ms
0.01-0.10ms ######### 879012 (100.0%) avg: 0.016ms
0.10-1.00ms # 157 (0.0%) avg: 0.155ms
1.00-10.00ms # 7 (0.0%) avg: 2.913ms
10.00-100.00ms # 1 (0.0%) avg: 11.457ms
>100.00ms 0 (0.0%) avg: 0.000ms

/Joel