Horton

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

Joel Jacobson <joel@compiler.org>

View thread
From: "Joel Jacobson" <joel@compiler.org>
To: "Matheus Alcantara" <matheusssilv97@gmail.com>, "Masahiko Sawada" <sawada.mshk@gmail.com>
Cc: Álvaro Herrera <alvherre@kurilemu.de>, "Arseniy Mukhin" <arseniy.mukhin.dev@gmail.com>, "Rishu Bagga" <rishu.postgres@gmail.com>, "Yura Sokolov" <y.sokolov@postgrespro.ru>, "Daniil Davydov" <3danissimo@gmail.com>, "Alexandra Wang" <alexandra.wang.oss@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, "Tom Lane" <tgl@sss.pgh.pa.us>
Date: 2025-10-22T00:02:08Z
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 Wed, Oct 22, 2025, at 02:16, Matheus Alcantara wrote:
>> Regarding the v8 patch, it introduces a fundamentally new way of
>> managing notification entries (adding entries with 'committed' state
>> and marking them 'aborted' in abort paths). This affects all use
>> cases, not just those involving very old unconsumed notifications, and
>> could introduce more serious bugs like PANIC or SEGV. For
>> backpatching, I prefer targeting just the problematic behavior while
>> leaving unrelated parts unchanged. Though Álvaro might have a
>> different perspective on this.
>>
> Thanks very much for this explanation and for what you've previously
> wrote on [1]. It's clear to me now that the v8 architecture is not a
> good way to go.

How about doing some more work in vac_update_datfrozenxid()?

Pseudo-code sketch:

```
void
vac_update_datfrozenxid(void)
{

    /* After computing newFrozenXid from all known sources... */

    TransactionId oldestNotifyXid = GetOldestQueuedNotifyXid();

    if (TransactionIdIsValid(oldestNotifyXid) &&
        TransactionIdPrecedes(oldestNotifyXid, newFrozenXid))
    {
        /*
         * The async queue has XIDs older than our proposed freeze point.
         * Attempt cleanup, then back off and let the next VACUUM benefit.
         */

        if (asyncQueueHasListeners())
        {
            /*
             * Wake all listening backends across *all* databases
             * that are not already at QUEUE_HEAD.
             * They'll hopefully process notifications and advance
             * their pointers, allowing the next VACUUM to freeze further.
             */
            asyncQueueWakeAllListeners();
        }
        else
        {
            /*
             * No listeners exist - discard all unread notifications.
             * The next VACUUM should succeed in advancing datfrozenxid.
             * asyncQueueAdvanceTailNoListeners() would take exclusive lock
             * on NotifyQueueLock before checking
             * QUEUE_FIRST_LISTENER == INVALID_PROC_NUMBER
             */
            asyncQueueAdvanceTailNoListeners();
        }

        /*
         * Back off datfrozenxid to protect the old XIDs.
         * The cleanup we just performed should allow the next VACUUM
         * to freeze further.
         */
        newFrozenXid = oldestNotifyXid;
    }
}
```

Maybe it wouldn't solve all problematic situations, but to me it seems
like these measures could help many of them, or am I missing some
crucial insight here?

/Joel