Re: Proposal: Out-of-Order NOTIFY via GUC to Improve LISTEN/NOTIFY Throughput

Hi,

On Fri, Sep 5, 2025 at 1:53 AM Rishu Bagga <rishu(dot)postgres(at)gmail(dot)com> wrote:
>
> Attached is an initial patch that implements this idea.

Thank you for sharing the patch and for working on this!

I briefly read the patch and there are some points:

1) There are some compilation warnings about unused functions.

2) I thought the idea was to add notification_data_entry_lsn to the
transaction's commit wal entry, but the patch introduced the new
XLOG_ASYNC_NOTIFY_COMMIT wal entry and put lsn in it. Also
XLOG_ASYNC_NOTIFY_COMMIT is written in AtCommit_Notify, when a
transaction is already committed, so we write it after the commit
entry. I don't think we can write anything about the transaction to
the wal after we wrote the transaction commit record. In my
understanding all transaction stuff in wal should be before commit /
abort entries. At least because during committing we sync wal with
disk up to the commit entry, and we can't guarantee that anything
after a commit entry will survive a crash. Anyway I think we just
don't need this new wal entry.

3) I see you removed global lock, but I don't see where you use lock
for step 3 [0] to provide the commit order of notifications. I thought
step 3 should look something like that:

if (have_notifications)
acquire_lock();

...

XactLogCommitRecord();

....

if (have_notifications)
{
asyncQueueAddCompactEntry();
release_lock();
}

So this way we will have the same order for commit records in wal and
entries in the listen/notify queue. I'm not sure what level we should
add this stuff, but it seems that to minimise the time we hold the
lock we need to release it before calling XLogFlush(). I'm not sure
about it.

Best regards,
Arseniy Mukhin