Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue

On Thu Aug 21, 2025 at 8:14 PM -03, Matheus Alcantara wrote:
> I'll work on this considering the initial Daniil comments at [1]
>
> [1] https://www.postgresql.org/message-id/CAJDiXgg1ArRB1-6wLtXfVVnQ38P9Y%2BCDfEc9M2TXiOf_4kfBMg%40mail.gmail.com
>
I've been working on this on the last few days, please see the attached
patch version.

In this new version I tried to follow the suggestion from Daniil of
scanning all pages from tail to head of the async queue.

I tried to use other data structures like a list or a hashmap to store
the xids but it shows to me more complicated due to duplicated
information being stored and also considering that this data structure
should be stored on shared memory, so I just tried to get the oldest xid
with the information that we already have on the async queue.

To find the oldest xid on the async queue I basically read all
AsyncQueueEntry's on async queue pages on SLRU cache from tail to head,
skipping only uncommitted transactions and notifications for different
databases.

The code that reads the pages and the entries within the page is a bit
coupled with the send notification logic. The important functions that
execute this is asyncQueueReadAllNotifications() which read the pages
from SLRU cache and then call the asyncQueueProcessPageEntries() which
read the entries within the page and send the notification to the client
if needed. Since I need a very similar code to read the notification
entries on the queue to check the min xid I think that it would be good
to have an API that just works like an iterator and returns the next
queue entry to work on so that it can be used for both cases.

In this patch I created an AsyncQueueIterator that iterates over pages
and AsyncQueueEntry's within the pages. The code is based on the
asyncQueueReadAllNotifications() and asyncQueueProcessPageEntries()
functions. For now I'm just implementing the iterator logic and use at
AsyncQueueMinXid() to get the min xid on the queue to make the review
more easier but I think that we can have another patch that refactor
asyncQueueReadAllNotifications() and asyncQueueProcessPageEntries()
functions to use this iterator approach and avoid duplicated code.

I'm also adding a TAP test that reproduces the issue, but I'm not sure if
it's declared on the best path.

I think that it would be good to perform some benchmark tests to see if
we have performance issues when reading very long async queues.

Thoughts?

--
Matheus Alcantara