Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section
| От | Joel Jacobson |
|---|---|
| Тема | Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section |
| Дата | |
| Msg-id | 403ba8bd-820d-47ae-b676-3bdedfa4cfce@app.fastmail.com обсуждение исходный текст |
| Ответ на | Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section (Tom Lane <tgl@sss.pgh.pa.us>) |
| Ответы |
Re: [PATCH] Avoid pallocs in async.c's SignalBackends critical section
|
| Список | pgsql-hackers |
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
В списке pgsql-hackers по дате отправления: