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
