Horton

Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue

Heikki Linnakangas <hlinnaka@iki.fi>

View thread
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Joel Jacobson <joel@compiler.org>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-11-03T11:02:32Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Clear 'xid' in dummy async notify entries written to fill up pages

  2. Fix remaining race condition with CLOG truncation and LISTEN/NOTIFY

  3. Fix bug where we truncated CLOG that was still needed by LISTEN/NOTIFY

  4. Escalate ERRORs during async notify processing to FATAL

  5. Limit the size of TID lists during parallel GIN build

Attachments

On 31/10/2025 18:40, Heikki Linnakangas wrote:
> On 31/10/2025 01:27, Joel Jacobson wrote:
>> On Fri, Oct 31, 2025, at 00:08, Joel Jacobson wrote:
>>> On Thu, Oct 30, 2025, at 14:25, Heikki Linnakangas wrote:
>>>> Joel, since you've been working on some optimizations in this area too,
>>>> would you happen to have some suitable performance test scripts for 
>>>> this?
>>>
>>> Glad you asked. I'm actually working on a benchmark+correctness tester.
>>> It's very much work-in-progress though, don't look too much at the code,
>>> or your eyes will bleed.
>>>
>>> It's a combined benchmark + correctness tester, that verifies that only
>>> the expected notifications are received on the expected connections,
>>> while at the same time doing timing measurements.
>>
>> To run multiple pg_bench_lino processes in parallell to simulate
>> concurrent workloads, I realized the randomization of the channel names
>> and payloads were not random enough to avoid collissions. New version
>> attached that uses real UUIDs for channel names and payloads.
> 
> Thanks! Here's a sketch for holding the bank lock across 
> TransactionIdDidCommit() calls. In quick testing with your test program, 
> I can't see any performance difference. However, I'm not quite sure what 
> options I should be using to stress this. My gut feeling is that it's 
> fine, but it'd be nice to do construct a real worst case test case to be 
> sure.

I wrote another little stand-alone performance test program for this, 
attached. It launches N connections that send NOTIFYs to a single 
channel as fast as possible, and M threads that listen for the 
notifications. I ran it with different combinations of N and M, on 
'master' and on REL_14_STABLE (which didn't have SLRU banks) and I 
cannot discern any performance difference from these patches. So it 
seems that holding the SLRU (bank) lock across the 
TransactionIdDidCommit() calls is fine.

- Heikki