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

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