Horton

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

Matheus Alcantara <matheusssilv97@gmail.com>

View thread
From: Matheus Alcantara <matheusssilv97@gmail.com>
To: Yura Sokolov <y.sokolov@postgrespro.ru>, Masahiko Sawada <sawada.mshk@gmail.com>
Cc: 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-03T21:04:47Z
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

Thanks for the comments!

On Tue Sep 2, 2025 at 4:37 AM -03, Yura Sokolov 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.
>
Maybe we could have a boolean "committed" field on AsyncQueueEntry and
check this field before sending the notification instead of call
TransactionIdDidCommit()? Is something similar what you are proposing?

We create the AsyncQueueEntry's and add into the SLRU at
PreCommit_Notify(), so to mark these entries as committed on
AtCommit_Notify() we would need a way to find these entries on the SLRU
from the pendingNotifies->events that contains the notifications added
on the current transaction being committed.

This idea looks promising to me TBH, I'm just not sure if it's
completely safe to mark an AsyncQueueEntry on the queue as committed
without checking on TransactionIdDidCommit(). I would like to read more
opinions about this before working on the next version based on this
idea.

--
Matheus Alcantara