Thread

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

Joel Jacobson <joel@compiler.org> — 2025-11-25T18:14:12Z

On Tue, Nov 25, 2025, at 16:43, Joel Jacobson wrote:
> On Tue, Nov 25, 2025, at 11:15, Joel Jacobson wrote:
>> With the following three changes, I think the only remaining
>> potentially-risky code in AtCommit_Notify, is the acquire/release of
>> locks.
>
> Patch 0001 and 0002 are unchanged since v2.
>
> 0003:
>
> Since this thread is specifically about avoiding pallocs in the
> effective "critical section", I realize we shouldn't change
> listenChannels from a list to hash (in this patch), but just move the
> existing potentially-risky code out of AtCommit_Notify.
>
> Thanks to a sharper focus on that, I realized Tom's alternative design
> idea from [1], to just go ahead and perform the LISTEN/UNLISTEN updates
> in PreCommit_Notify, is an excellent simplification, with no real
> downsides that I can identify.
>
> This allowed simplifying 0003 a lot, by just doing all LISTEN/UNLISTEN
> operations during PreCommit:

Darn, I forgot about the edge-case where something fails in xact.c
in between PreCommit_Notify(); and AtCommit_Notify();
causing an abort.

In this case, AtAbort_Notify() would need to undo the effect
of the LISTEN/UNLISTEN commands that would already have been
executed, with the 0003 patch.

A normal BEGIN; LISTEN foo; ROLLBCK; is not a problem though,
since that will never reach PreCommit_Notify().

Just curious, what type of problems could cause an abort between
PreCommit_Notify and AtCommit_Notify?

I wonder if we can think of a clever and cheap way to do the
cleanup in AtAbort_Notify(), without needing the boolean flag?

I still believe 0001 and 0002 are correct though.

/Joel