Hi,
Thank you for working on it! Benchmarking looks great. There are several points:
I tried the patch and it seems listeners sometimes don't receive
notifications. To reproduce it you can try to listen to the channel in
one psql session and send notifications from another psql session. But
all tests are fine, so I tried to write a TAP test to reproduce it. It
passes on master and fails with the patch, so looks like it's real.
Please find the repro in attachments. I added the TAP test to amcheck
module just for simplicity.
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(). asyncQueueAdvanceTail() doesn't
know that listeners have a local copy of their positions and can
truncate the queue beyond that which means listeners can try to read
notifications from the truncated segment. I managed to reproduce it
locally. Please let me know if more details are needed.
BTW error message a bit confusing:
2025-10-15 13:32:15.570 MSK [261845] ERROR: could not access status
of transaction 0
2025-10-15 13:32:15.570 MSK [261845] DETAIL: Could not open file
"pg_notify/000000000000001": No such file or directory.
Looks like all slru IO errors have an error message about transaction
status. It's not a problem really as we have a directory path in the
log.
Best regards,
Arseniy Mukhin
