Feedback-ID: ic6394509:Fastmail
MIME-Version: 1.0
Date: Wed, 15 Oct 2025 17:36:11 +0200
From: "Joel Jacobson" <joel@compiler.org>
To: "Chao Li" <li.evan.chao@gmail.com>, "Tom Lane" <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Message-Id: <0a5a20d3-4621-46b3-b2ab-903f63a20dea@app.fastmail.com>
In-Reply-To: <1F7227F5-C33D-4E2C-8511-33F1468590D0@gmail.com>
References: <6899c044-4a82-49be-8117-e6f669765f7e@app.fastmail.com>
 <165530.1752362320@sss.pgh.pa.us>
 <02a7cd37-e2fc-4212-8b19-f8c239c95fb8@app.fastmail.com>
 <e396ecf3-4227-4918-b9ff-e9568dcebcf0@app.fastmail.com>
 <b6a427fe-2ed6-4568-85d9-207a68172617@app.fastmail.com>
 <CAK80=jhmE40KVqQ3ho37MArS7cAED1p9m7uikDxcnDmqdW7t8A@mail.gmail.com>
 <96f00bf1-cc9d-4520-9d02-9e14e7767c88@app.fastmail.com>
 <30c2aa7d-dd6c-4b68-a2e4-f217a1a34acf@app.fastmail.com>
 <CA+hUKGLrMGkWDB0cwTa0RqD+AF7O-Ywgck8aVYKwOQnZgYRRug@mail.gmail.com>
 <0b4d402a-9ac2-4aa8-acf8-8231dbe579ea@app.fastmail.com>
 <c8e84bb2-d2e5-4a77-b547-3a93c5778e7e@app.fastmail.com>
 <3095599.1758644879@sss.pgh.pa.us>
 <0dc6a2cc-5216-4dc1-9dd2-430cafc6095b@app.fastmail.com>
 <E7D8A5CD-C445-4713-B6B7-6CC1D3A71161@gmail.com>
 <baf71275-9083-41be-ae13-f80821541a1a@app.fastmail.com>
 <52CC167F-763B-4ECA-B0B4-DAB381816828@gmail.com>
 <d35a114d-204a-4b40-9284-06749532b724@app.fastmail.com>
 <9186C6D0-F7A9-482A-9183-89E530B57E36@gmail.com>
 <da0996!!!!!d8-ecf9-490d-bbd0-2a5bdb5b115a@app.fastmail.com>
 <D1CF7CA5-213A-4FC1-86DB-052D58904A0A@gmail.com>
 <cad78ff4-aae9-4c1c-8e16-ff0ccd4957f1@app.fastmail.com>
 <1073593.1759423179@sss.pgh.pa.us>
 <4bd5e6c4-6fa7-44bb-869d-59a32a331fa8@app.fastmail.com>
 <CAFY6G8dap-bCnAnMG-2Gzew8yv2Vbi9gsx9+yszKMmd57ygfvA@mail.gmail.com>
 <85828f29-e72e-4400-94f3-9a69bc8dc239@app.fastmail.com>
 <2495353.1759860890@sss.pgh.pa.us>
 <8aeae418-92a6-4bbd-9c06-9574c79e59f7@app.fastmail.com>
 <2531672.1759868124@sss.pgh.pa.us>
 <474efa78-337c-41cd-a73a-f845a0115109@app.fastmail.com>
 <2749343.1759949176@sss.pgh.pa.us>
 <8bfca2be-1ec0-4e15-aafb-0b7b661fe936@app.fastmail.com>
 <9eba307f-f2fb-48f0-9507-2e197f39ef9e@app.fastmail.com>
 <cde2a91d-d47a-43b5-ab27-f98aa739d38e@app.fastmail.com>
 <8c71183a-0d28-4bcf-a806-78446ff95404@app.fastmail.com>
 <1009807.1760476747@sss.pgh.pa.us>
 <1F7227F5-C33D-4E2C-8511-33F1468590D0@gmail.com>
Subject: Re: Optimize LISTEN/NOTIFY
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/0a5a20d3-4621-46b3-b2ab-903f63a20dea%40app.fastmail.com>
Precedence: bulk

On Wed, Oct 15, 2025, at 05:19, Chao Li wrote:
>  * B enters PreCommit_Notify(), it gets the NotifyQueueLock first, it=20
> records headBeforeWrite =3D 1 and writes to 3, and records headAfterWr=
ite=20
> =3D 3.
>  * Now QueueHead is 3.
>  * C enters PreCommit_Notify(),  it records headBeforeWrite =3D 3 and=20
> writes to 5, and records headAfterWrite =3D 5.

No, when C enters PreCommit_Notify, it will be waiting on the
heavyweight lock, currently held by B, which B will hold
until it commits. It will then see headBeforeWrite =3D 3.

>  * Now QueueHead is 5
>  * C starts to run AtCommit_Notify(), as A=E2=80=99s head is 1, doesn=E2=
=80=99t equal=20
> to C=E2=80=99s headBeforeWrite, C won=E2=80=99t advance A=E2=80=99s he=
ad.
>  * A starts to run AtCommit_Notify(), A=E2=80=99s head equals to B=E2=80=
=99s=20
> beforeHeadWrite, B will advance A=E2=80=99s head to 3.

No, like explained above, B cannot be running here,
it must have committed already (or aborted) since C
was waiting on the heavyweight lock held by B.

The example therefore seems invalid to me.

> I agree with Tom that GetPendingNotifyChannels() is too heavy and unne=
cessary.
>
> In PreCommit_Notify(), we can maintain a local hash table to record=20
> pending nofications=E2=80=99 channel names. dahash also supports hash =
table in=20
> local memory.

I'm confused, I assume you mean "dynahash" since there is no "dahash"
in the sources? I see dynahash has local-to-a-backend support,
but I don't see why we would need a hash table for this,
we just iterate over it once in SignalBackends,
I think the local list is fine.

The latest version gets rid of GetPendingNotifyChannels()
and replaces it with the local list pendingNotifyChannels.

> And the local static numChannelsListeningOn is also not needed. We can=20
> get the count from the local hash.

No, you're mixing up the data structures.
The local hash you suggested was for pending notify channels,
but numChannelsListeningOn was needed when we didn't have
listenChannels. Now that I've reverted back to listenChannels,
I also replaced `(numChannelsListeningOn =3D=3D 0)`
with `(listenChannels =3D=3D NIL)`.

> WRT to v6, I got a few new comments:
...
> In this comment, you refer to =E2=80=9CchannelHash=E2=80=9D and =E2=80=
=9Cthe shared channel=20
> hash table=E2=80=9D, they are the same thing, but easy to make readers=
 to=20
> misunderstand.

Right, will try to improve this in the next version.

>  pg_listening_channels(PG_FUNCTION_ARGS)
>  {
>  FuncCallContext *funcctx;
> + List   *listenChannels;
...
> listenChannels is only used within the =E2=80=9Cif=E2=80=9D, so it=E2=80=
=99s definition can be=20
> moved into the =E2=80=9Cif=E2=80=9D.

Comment not applicable since local variable listenChannels has now been
removed from pg_listening_channels, now using the original static
listenChannels instead.

> + /* Check for lagging backends when the queue spans multiple pages */
> + if (queue_length > 0)
...
> I wonder why this check is needed. If queue_length is 0, can we return=20
> immediately from SignalBackends()?

This check has been removed in the latest version.

/Joel