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

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Joel Jacobson <joel(at)compiler(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue
Date: 2025-10-31 16:40:03
Message-ID: dc6b131d-b010-401e-951e-48535bfb5861@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

There are some opportunities for micro-optimizations here:

* IsListeningOn() is kind of expensive if a backend is listening
multiple channels. We really should turn that into a hash table. As the
patch stands, I'm doing the IsListeningOn() calls while holding the bank
lock, but unless we speed up IsListeningOn() in those degenerate cases,
we should perhaps call it only after copying and releasing the lock.

* If IsListeningOn() is made fast, it might make sense to call it before
TransactionIdDidCommit().

* Implement the hint bits.

I don't know how much those matter. Again, a test case would be nice.

I'll work more on performance testing next week of this, if no one else
picks that up.

- Heikki

Attachment Content-Type Size
0001-Fix-bug-where-we-truncated-CLOG-that-was-still-neede.patch text/x-patch 14.0 KB
0002-Hold-SLRU-bank-lock-across-TransactionIdDidCommit-in.patch text/x-patch 6.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabrice Chapuis 2025-10-31 16:45:33 Re: Issue with logical replication slot during switchover
Previous Message Jacob Champion 2025-10-31 16:24:17 Re: libpq: Bump protocol version to version 3.2 at least until the first/second beta