Re: Optimize LISTEN/NOTIFY

From: "Joel Jacobson" <joel(at)compiler(dot)org>
To: "Chao Li" <li(dot)evan(dot)chao(at)gmail(dot)com>, "Arseniy Mukhin" <arseniy(dot)mukhin(dot)dev(at)gmail(dot)com>
Cc: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Optimize LISTEN/NOTIFY
Date: 2025-10-26 06:33:32
Message-ID: 55d24cbb-e9ef-491f-a99b-b3dbd7cecdf9@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joel Jacobson 2025-10-26 07:08:42 Re: Optimize LISTEN/NOTIFY
Previous Message Chao Li 2025-10-26 04:11:25 Re: Optimize LISTEN/NOTIFY