public inbox for [email protected]  
help / color / mirror / Atom feed
From: Joel Jacobson <[email protected]>
To: Chao Li <[email protected]>
To: Arseniy Mukhin <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Optimize LISTEN/NOTIFY
Date: Sun, 26 Oct 2025 07:33:32 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAK80=jhmE40KVqQ3ho37MArS7cAED1p9m7uikDxcnDmqdW7t8A@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CA+hUKGLrMGkWDB0cwTa0RqD+AF7O-Ywgck8aVYKwOQnZgYRRug@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAFY6G8dap-bCnAnMG-2Gzew8yv2Vbi9gsx9+yszKMmd57ygfvA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAE7r3MLivh1sHWF06hrVXkiQbw-KChPcQsh+9CheXprm5vRVMQ@mail.gmail.com>
	<[email protected]>
	<CAE7r3MK-3AOdh1mpZ8hw9h6F_i0D5RMoAy7CttnfCJRpB8GJDA@mail.gmail.com>
	<[email protected]>

On Sun, Oct 26, 2025, at 05:11, Chao Li wrote:
> I figured out a way to resolve the race condition for alt3:
>
> * add an awakening flag for every listener, this flag is only set by 
> listeners
> * add an advisory pos for every listener, similar to alt1
> * if a listener is awaken, notify only updates the listener’s advisory 
> pos; otherwise directly advance its position.
> * If a running listener see current pos is behind advisory pos, then 
> stop reading
>
> See more details in attach patch file, I added code comments for my 
> changes. Now the TAP test won’t hit the race condition. 
> ```
> # +++ tap check in src/test/authentication +++
> t/008_listen-pos-race.pl .. skipped: Injection points not supported by 
> this build
> Files=1, Tests=0,  0 wallclock secs ( 0.00 usr  0.00 sys +  0.03 cusr  
> 0.01 csys =  0.04 CPU)
> Result: NOTESTS
> ```
>
> And with my solution, listeners longer will still use local pos, so 
> that no longer need to acquire notification lock in every loop.

This sounds promising, similar to what I had in mind. I was thinking
about the idea of using the advisoryPos only when the listening backend
is known to be running (which felt like it would need another shared
boolean field), and to move its pos field directly only when it's not
running, since if it's running we don't need to optimize for context
switching, since it's by definition already running.

What I wanted to investigate what all the concurrency situations
that we can imagine, i.e. to permutate all possible differences
we can think of into a truth table, and reason about each case.

The ones I can think of are, from the perspective of SignalBackends,
reasoning about a specific listening backend:

{is interested in the notifications, is not interested in the notifications} x
{wakeupPending=false, wakeupPending=true} x
{pos < queueHeadBeforeWrite, pos == queueHeadBeforeWrite, pos > queueHeadBeforeWrite, pos == queueHeadAfterWrite, pos > queueHeadAfterWrite} x
{is running, is not running}

This gives 2x2x5x2=40 states to reason about. Some of these combinations
are probably impossible, I still think it would be good to include them
and explain why they are impossible.

> The patch stack is: v20 patch -> alt3 patch -> tap patch -> my patch. 
> Please see if my solution works.
>
> I also made a tiny change in the TAP script to allow it to terminate gracefully.

I haven't looked at the code yet, tried to apply the patch but it fails:

shasum of files:
```
ca54ffa02ac54efd65acce0d09b18e630b5d7982  0001-optimize_listen_notify-v20.patch
5755701bb0e7ac7a0cea3abab9d74a0001b7b63a  0002-optimize_listen_notify-v20.patch
5819e23b5760023be70d2582207b72164904e952  0002-optimize_listen_notify-v20-alt3.txt
33d700dc0b3288d46705e85d381cb564d99079d1  0001-TAP-test-with-listener-pos-race.patch.nocfbot
8ee716451bd5f85761b666712bdfd8b5d936f92d  fix-race.patch
```

Trying to apply them on top of current master (39dcfda2d23ac39f14ecf4b83e01eae85d07d9e5):

```
% git apply 0001-optimize_listen_notify-v20.patch
% git apply 0002-optimize_listen_notify-v20.patch
% git apply 0002-optimize_listen_notify-v20-alt3.txt
% git apply 0001-TAP-test-with-listener-pos-race.patch.nocfbot
% git apply fix-race.patch
fix-race.patch:100: indent with spaces.
					     (QUEUE_POS_PRECEDES(queueHeadBeforeWrite, pos) && QUEUE_POS_PRECEDES(pos, queueHeadAfterWrite))) &&
error: patch failed: src/backend/commands/async.c:250
error: src/backend/commands/async.c: patch does not apply
error: patch failed: src/test/authentication/t/008_listen-pos-race.pl:8
error: src/test/authentication/t/008_listen-pos-race.pl: patch does not apply
```

I'll try to resolve it manually, but in case you're quicker to reply, I'm sending this now.

/Joel





reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Optimize LISTEN/NOTIFY
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox