Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section

From: "Joel Jacobson" <joel(at)compiler(dot)org>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Heikki Linnakangas" <hlinnaka(at)iki(dot)fi>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section
Date: 2025-11-25 10:15:58
Message-ID: 054d3648-f788-4f12-8185-d9d0517dc5d4@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Nov 24, 2025, at 22:53, Joel Jacobson wrote:
> On Mon, Nov 24, 2025, at 17:06, Tom Lane wrote:
>> Unfortunately, releasing locks, sending notifies, etc is not all
>> that "noncritical" if you want the DB to keep functioning well.
>> But there's a good deal of code in there and making it all obey
>> the critical-section rules looks painful.
>
> I see why a critical-section is probably too painful. But since the
> direction in [1] is to avoid adding new possibly risky operations to
> AtCommit_Notify, I don't think it's completely unreasonable to consider
> moving some existing ones into PreCommit_Notify, when feasible.
>
> If it's preferable, I'm fine dropping this standalone patch and folding
> any such adjustments into v29 in [1], or I can just leave the existing
> code untouched?

With the following three changes, I think the only remaining
potentially-risky code in AtCommit_Notify, is the acquire/release of
locks.

* 0001-async-avoid-pallocs-in-critical-section-v2.patch:
Preallocate signal arrays to avoid pallocs AtCommit.

* 0002-async-avoid-pallocs-in-critical-section-v2.patch:
Move asyncQueueAdvanceTail from AtCommit to PreCommit.

* 0003-async-avoid-pallocs-in-critical-section-v2.patch
Convert listenChannels to hash table.
This is based on Heikki's suggestion

"We really should turn that into a hash table."

from the bug fix thread [2] combined with Tom's idea of a boolean

"is it REALLY listening?"

field [1].

Together, these patches allows us to gets rid of the following comments:

0001:
- * XXX in principle these pallocs could fail, which would be bad. Maybe
- * preallocate the arrays? They're not that large, though.

0002:
- * This is (usually) called during CommitTransaction(), so it's important for
- * it to have very low probability of failure.

0003:
- * XXX It is theoretically possible to get an out-of-memory failure here,
- * which would be bad because we already committed. For the moment it
- * doesn't seem worth trying to guard against that, but maybe improve this
- * later.

Please advise if we want these changes, and if so, if they should be
folded into [1] i.e. closing this thread, or if we want to keep this thread.

/Joel

[1] https://www.postgresql.org/message-id/flat/6899c044-4a82-49be-8117-e6f669765f7e(at)app(dot)fastmail(dot)com

[2] https://www.postgresql.org/message-id/flat/CAK98qZ3wZLE-RZJN_Y%2BTFjiTRPPFPBwNBpBi5K5CU8hUHkzDpw%40mail.gmail.com

Attachment Content-Type Size
0001-async-avoid-pallocs-in-critical-section-v2.patch application/octet-stream 3.6 KB
0003-async-avoid-pallocs-in-critical-section-v2.patch application/octet-stream 13.9 KB
0002-async-avoid-pallocs-in-critical-section-v2.patch application/octet-stream 2.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2025-11-25 10:30:11 RE: How can end users know the cause of LR slot sync delays?
Previous Message Heikki Linnakangas 2025-11-25 10:07:27 Re: POC: make mxidoff 64 bits