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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Joel Jacobson <joel(at)compiler(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section
Date: 2025-11-24 16:06:12
Message-ID: 954729.1764000372@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
> On 23/11/2025 16:45, Joel Jacobson wrote:
>> This patch addresses this comment in async.c's SignalBackends:
>> * XXX in principle these pallocs could fail, which would be bad.
>> * Maybe preallocate the arrays? They're not that large, though.

> Ugh. I wonder if we should put an actual critical section around those
> post-commit cleanup actions? As you said, it's effectively a critical
> section already, except that we don't get the benefit of the
> AssertNotInCriticalSection assertions.
> Or even better, could we move things out of that effective critical
> section? It's painful to write code that cannot palloc.

I don't think Joel did anybody any favors by separating this patch
fragment from its larger context [1]. Given the infrequency of
complaints about failures in this area, I'm not sure that the
notational pain of an actual critical section is justified.
But I complained that the changes contemplated in [1] were raising
the probability of failure, and while working on tamping that back
down we decided to do something about this old gripe too.

There's a relevant comment in CommitTransaction():

* This is all post-commit cleanup. Note that if an error is raised here,
* it's too late to abort the transaction. This should be just
* noncritical resource releasing.

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.

regards, tom lane

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-11-24 16:09:29 Re: get rid of Pointer type, mostly
Previous Message Jim Jones 2025-11-24 15:51:25 Re: Add notification on BEGIN ATOMIC SQL functions using temp relations