| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Arseniy Mukhin <arseniy(dot)mukhin(dot)dev(at)gmail(dot)com> |
| Cc: | Joel Jacobson <joel(at)compiler(dot)org>, 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 04:11:25 |
| Message-ID: | 0BCA1C2D-B92C-459E-B1A6-6D06BA4C62CF@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Oct 23, 2025, at 18:02, Arseniy Mukhin <arseniy(dot)mukhin(dot)dev(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Oct 23, 2025 at 11:17 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>>
>>
>>> On Oct 21, 2025, at 00:43, Arseniy Mukhin <arseniy(dot)mukhin(dot)dev(at)gmail(dot)com> wrote:
>>>
>>>
>>> I managed to reproduce the race with v20-alt3. I tried to write a TAP
>>> test reproducing the issue, so it was easier to validate changes.
>>> Please find the attached TAP test. I added it to some random package
>>> for simplicity.
>>>
>>
>> With alt3, as we have acquired the notification lock after reading every message to update the POS, I think we can do a little bit more optimization:
>>
>> The notifier: in SignalBackend()
>> * Now we check if a listener’s pos equals to beforeWritePos, then we do “directly advancement”
>> * We can change to if a listener’s pos is between beforeWritePos and afterWritePos, then we can do the advancement.
>>
>> The listener: in asyncQueueReadAllNotifications():
>> * With alt3, we only lock and update pos
>> * We can do more. If current pos in shared memory is after that local pos, then meaning some notifier has done an advancement, so it can stop reading.
>>
>
> I think this would be a reasonable optimization if it weren't for the
> race condition mentioned above. The problem is that if the local pos
> lags behind the shared memory pos, it could point to a truncated queue
> segment, so we shouldn't allow that.
>
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.
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.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| fix-race.patch | application/octet-stream | 7.2 KB |
| unknown_filename | text/plain | 4 bytes |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joel Jacobson | 2025-10-26 06:33:32 | Re: Optimize LISTEN/NOTIFY |
| Previous Message | David Rowley | 2025-10-26 01:25:48 | Re: another autovacuum scheduling thread |