On Mon, Nov 24, 2025, at 17:06, Tom Lane wrote:
> I don't think Joel did anybody any favors by separating this patch
> fragment from its larger context [1].
I'm a bit surprised by this. My intention in splitting it out
was based on earlier advice in [1] that I think made a lot of sense:
>> [...my idea of a bgworker to kick lagging backends...]
> If you feel that that's not robust enough,
> you should split it out as a separate patch that's advertised as a
> robustness improvement not a performance improvement, and see if you
> can get anyone to bite.
In general, I think it's nice when a bigger change can be split into
smaller meaningful committable changes, which seemed possible in this
case.
Heikki also raised a point that seems worth exploring:
AtCommit_Notify currently calls asyncQueueAdvanceTail.
After PreCommit_Notify, we already know whether tryAdvanceTail is
needed, so it looks feasible to move the asyncQueueAdvanceTail call to
the end of PreCommit_Notify.
> 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.
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?
/Joel
