| 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 "critical section" |
| Date: | 2025-11-25 18:14:12 |
| Message-ID: | b7ac2fd8-6f00-4082-936d-984a8ea267f2@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2025-11-25 18:14:56 | Re: reduce size of logs stored by buildfarm |
| Previous Message | Álvaro Herrera | 2025-11-25 17:53:05 | Re: The pgperltidy diffs in HEAD |