From: Joel Jacobson <joel@compiler.org>
To: Arseniy Mukhin <arseniy.mukhin.dev@gmail.com>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: Matheus Alcantara <matheusssilv97@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Rishu Bagga <rishu.postgres@gmail.com>
Cc: Yura Sokolov <y.sokolov@postgrespro.ru>
Cc: Daniil Davydov <3danissimo@gmail.com>
Cc: Alexandra Wang <alexandra.wang.oss@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue
Date: Sat, 18 Oct 2025 07:43:15 +0200
Message-ID: <01fbde80-0ff6-4bf4-b942-cd60b76380d1@app.fastmail.com> (raw)
In-Reply-To: <CAE7r3MKcd0xHdGL-47MkgumZ0Ebwib5V2+3TkG4ei6zKXXyJfg@mail.gmail.com>
References: <CAE7r3MLoc9uDO2b542i_ZiksrB_iyvO1NKSzS7MOLdGzMuG=Kw@mail.gmail.com>
<202510171347.twmkbitfsgfh@alvherre.pgsql>
<CAE7r3MKcd0xHdGL-47MkgumZ0Ebwib5V2+3TkG4ei6zKXXyJfg@mail.gmail.com>
On Fri, Oct 17, 2025, at 22:50, Arseniy Mukhin wrote:
> On Fri, Oct 17, 2025 at 7:31 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>>
>> I have the impression that this thread has lost focus on the idea of
>> producing a backpatchable bugfix. The last proposed patch has a lot of
>> new mechanism that doesn't seem suitable for backpatch. I could be
>> wrong of course.
>>
>
> Oops, I guess the TAP test that I added in the last version and that
> uses injection points is one of those things. PFA the new version
> without it. I also noticed that CheckSharedObjectLockedByMe() which
> was introduced in one of previous versions is unused, so I removed it
> too.
What a funny coincidence that the approach in this patch,
has one similarity with the "Direct advancement" approach
in the patch in the "Optimize LISTEN/NOTIFY" [1] thread,
namely that we're both interested in QUEUE_HEAD before/after
we push the notifications into the queue, in PreCommit_Notify().
It looks to me like our new data structures are Interchangeable,
so I guess we probably want both patches to eventually settle
on one and the same?
The differences I note between our queue head before/after code are:
- In this patch, you are palloc'ing a struct with two fields.
In [1], we're using two separate static QueuePosition variables.
- In this patch, you are taking/releasing a shared lock before/after
the loop to read QUEUE_HEAD and set previousHead/head.
In [1], we avoid the need of the shared lock, by doing the reads
within the existing exclusive lock inside the loop, but instead
therefore need a firstIteration bool, to know which is the first
iteration, and need to overwrite the after-var in each iteration.
I don't think the noted differences above matter, both seems fine.
Another thing I noticed in your patch that made me wonder,
is the naming of the new AsyncQueueEntry bool field,
which is given the name "committed".
I think this name is not entirely faithful, since when set to true,
the entry has not been committed yet.
How about negating the meaning of this boolean field?
To instead indicate when the entry has been rollbacked.
Then, it would clearly communicate just that.
Maybe naming it something like "rollbacked" or "aborted"?
/Joel
[1] https://www.postgresql.org/message-id/flat/6899c044-4a82-49be-8117-e6f669765f7e%40app.fastmail.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: joel@compiler.org, arseniy.mukhin.dev@gmail.com, alvherre@kurilemu.de, matheusssilv97@gmail.com, sawada.mshk@gmail.com, rishu.postgres@gmail.com, y.sokolov@postgrespro.ru, 3danissimo@gmail.com, alexandra.wang.oss@gmail.com, tgl@sss.pgh.pa.us
Subject: Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue
In-Reply-To: <01fbde80-0ff6-4bf4-b942-cd60b76380d1@app.fastmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox