Horton

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

Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>

View thread
From: Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Álvaro Herrera <alvherre@kurilemu.de>, Joel Jacobson <joel@compiler.org>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-11-07T09:41:48Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Clear 'xid' in dummy async notify entries written to fill up pages

  2. Fix remaining race condition with CLOG truncation and LISTEN/NOTIFY

  3. Fix bug where we truncated CLOG that was still needed by LISTEN/NOTIFY

  4. Escalate ERRORs during async notify processing to FATAL

  5. Limit the size of TID lists during parallel GIN build

On Fri, Nov 7, 2025 at 11:10 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 06/11/2025 17:13, Arseniy Mukhin wrote:
> > Let's say we have the queue:
> >
> > (tail ... pos1 ... bad_entry_pos ... head)
> >
> > bad_entry_pos - position of the entry where TransactionIdDidCommit fails.
> >
> > We have the listener L1 with pos = pos1. It means every new listener
> > should process the queue from pos1 (as it's max(listener's pos)) up to
> > the queue head and when they try to do it, they will fail on
> > 'bad_entry'.
>
> Gotcha.
>
> >> Another small change we could make is to check for listenChannels == NIL
> >> before calling TransactionIdDidCommit.
> >
> > There is a comment that describes why we need this initial reading in
> > Exec_ListenPreCommit:
> >
> >      /*
> >       * This is our first LISTEN, so establish our pointer.
> >       *
> >       * We set our pointer to the global tail pointer and then move it forward
> >       * over already-committed notifications.  This ensures we cannot miss any
> >       * not-yet-committed notifications.  We might get a few more but that
> >       * doesn't hurt.
> >
> > It seems that with such a check we will skip not-yet-committed
> > notifications and always get to the HEAD. If we decide that it's ok,
> > maybe we can just set 'pos' of every new listener to the HEAD without
> > reading?
>
> Right, I didn't mean skipping asyncQueueReadAllNotifications()
> altogether. Just the TransactionIdDidCommit() calls in it, when
> listenChannels == NIL, see attached patch. The idea is that when
> 'listenChannels == NIL', we're not going send the notification to the
> frontend regardless of what TransactionIdDidCommit says. If we don't
> call TransactionIdDidCommit, we won't fail on bad entries.
>

Ahh, now I see, thank you. WFM.


Best regards,
Arseniy Mukhin