[PG19] A canceled notify read leaves the NOTIFY queue stuck
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254258psql -h localhost -U postgresBuilt from patchset v1 (message #1), October 22, 2026 at 06:11 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t254258_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t254258_1 && git checkout t254258_1Patchset v1 (message #1) is on t254258_1
Hi,
If a listener fails while taking its snapshot in
asyncQueueReadAllNotifications(), its isAdvancing flag stays set. After
that no NOTIFY wakes or advances it, so the queue stops shrinking and
fills up. This is new in 19, from 282b1cde9de.
A cancel while the listener waits for a safe snapshot is enough:
L listens, W holds a serializable transaction, N sends notifications
set default_transaction_isolation = serializable; -- L
set default_transaction_read_only = on; -- L
set default_transaction_deferrable = on; -- L
listen cfg; -- L
begin isolation level serializable; select 1; -- W
begin isolation level read committed read write not deferrable; -- L
notify cfg; -- N
commit; -- L, waits on SafeSnapshot, cancel it
rollback; -- W
select pg_notify('other', repeat('x', 7000)); -- N, 16 times
The attached patch clears the flag on abort.
This is the third low item in my list [1]/messages/by-id/CAGRkXqSFdjqZP_kwRz7dv6LHX-CSrkDnynNXgntRsCEnQTKxmg@mail.gmail.com.
[1]: /messages/by-id/CAGRkXqSFdjqZP_kwRz7dv6LHX-CSrkDnynNXgntRsCEnQTKxmg@mail.gmail.com
/messages/by-id/CAGRkXqSFdjqZP_kwRz7dv6LHX-CSrkDnynNXgntRsCEnQTKxmg@mail.gmail.com
Thanks,
Shihao
shihao zhong <zhong950419@gmail.com> writes:
If a listener fails while taking its snapshot in
asyncQueueReadAllNotifications(), its isAdvancing flag stays set. After
that no NOTIFY wakes or advances it, so the queue stops shrinking and
fills up. This is new in 19, from 282b1cde9de.
Ooops.
The attached patch clears the flag on abort.
Playing around with this example, it avoids the queue-filling problem,
but there's still an undesirable aspect: the N process has sent the L
process a NOTIFY, but the L process does not see that, at least not
till another NOTIFY is sent (and then it reports both at once).
That's because the L process already cleared notifyInterruptPending,
so it won't re-enter ProcessIncomingNotify. This is measurably worse
than pre-v19: before, L would get the notify message when it next did
something, even without another NOTIFY. (I'm not immediately sure
why that changed.) The old behavior is still not great though;
ideally it'd report the notify right after finishing the aborted
COMMIT.
I tried to fix this by re-setting notifyInterruptPending in the new
block of code, but that's totally disastrous in the given example.
Having gotten out of the transaction, we immediately try to do
ProcessIncomingNotify again, and again we block in
GetTransactionSnapshot(). The user-visible effect is that you cannot
cancel the L process' COMMIT, even with multiple control-Cs, until the
W process commits or aborts. There might be other reasons why doing
this would turn into an endless failure loop, so on the whole that's
probably a bad idea. But I'd like to understand why v18 didn't
require receipt of a new NOTIFY before reporting the pending one.
Also, this business about GetTransactionSnapshot possibly blocking is
something that nobody foresaw when the async.c code was written.
I wonder if we should modify async.c to avoid it. Do we really need a
serializable snapshot for reading the notify queue? If we do, can we
acquire it before making any important state changes?
Anyway, the proposed patch is clearly an improvement so I'll go
ahead and push it. But we seem to have more to do here.
regards, tom lane
I wrote:
Playing around with this example, it avoids the queue-filling problem,
but there's still an undesirable aspect: the N process has sent the L
process a NOTIFY, but the L process does not see that, at least not
till another NOTIFY is sent (and then it reports both at once).
That's because the L process already cleared notifyInterruptPending,
so it won't re-enter ProcessIncomingNotify. This is measurably worse
than pre-v19: before, L would get the notify message when it next did
something, even without another NOTIFY. (I'm not immediately sure
why that changed.)
Oh, I must have fat-fingered the test case somehow, because when
I try this again, v18 behaves like HEAD. So never mind that bit.
I'm still interested in whether we can avoid blocking on a safe
snapshot, but that's clearly material for future development.
So we can close this open item for v19.
regards, tom lane
Hi Tom,
Thanks for committing that. Let's close the open item for v19.
I'm still interested in whether we can avoid blocking on a safe
snapshot, but that's clearly material for future development.
I can spend some time tonight to see what we can do about it.
Looks like I owe you quite a few patches, varno0, the WIN1252
validator, and this one. I will get them sorted out for you soon.
Thanks,
Shihao