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: Yura Sokolov <y.sokolov@postgrespro.ru>
Cc: Matheus Alcantara <matheusssilv97@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Daniil Davydov <3danissimo@gmail.com>, Álvaro Herrera <alvherre@kurilemu.de>, Alexandra Wang <alexandra.wang.oss@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-09-03T19:28:21Z
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

Hi,

On Tue, Sep 2, 2025 at 10:38 AM Yura Sokolov <y.sokolov@postgrespro.ru> wrote:
>
> 29.08.2025 01:31, Matheus Alcantara пишет:
> > 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.
>
> Since we still need to scan those pages, couldn't we mark finished
> transactions as committed/aborted?
> This way we may to not hold datfrozenxid for a long time and will allow
> both safe clog truncation and safe async queue notification.
> More over, most notifications could be marked as completed in
> AtCommit_Notify already. So there is a need to recheck a few notifications
> that were written but not marked in AtCommit_Notify.
>
> Since AsyncQueueEntry field is used only to check visibility, looks like it
> is safe to set it to FrozenTransactionId.

I like the idea to make queue records more self-sufficient, so we
don't need to check a transaction's status to determine if a
notification should be sent. What if we somehow mark every record of
an aborted transaction, so listeners can skip it without checking
transaction status? Also if the record is not marked as "aborted" we
also can send it without checking transaction status. IIUC
AtAbort_Notify is called before any listener can read records of the
aborted transaction (is it correct?), so we can do such "marking" in
AtAbort_Notify. And the listen/notify subsystem already has a similar
"marking" mechanism. At the end of every queue page we have a "dummy"
record which is skipped by every listener. Listeners skip it because
record's 'dboid' equals InvalidOid (listeners ignore any record where
the 'dboid' is different from their own). In the same way aborted
transactions can set 'dboid' to InvalidOid for their records in
AtAbort_Notify.

Some benefits of this approach:
1) Only aborted transactions would need to perform extra work.
2) Listeners don't need to check notify transaction status, but ofc
they still need to wait if the notify transaction is 'in progress'.
3) Vacuum don't need to be listen/notify aware.
4) We don't need to do an extra scan of the whole queue.

What do you think?


Best regards,
Arseniy Mukhin