Hi, On 2023-01-03 19:23:41 +0000, Peter Geoghegan wrote: > Delay commit status checks until freezing executes. > > pg_xact lookups are relatively expensive. Move the xmin/xmax commit > status checks from the point that freeze plans are prepared to the point > that they're actually executed. Otherwise we'll repeat many commit > status checks whenever multiple successive VACUUM operations scan the > same pages and decide against freezing each time, which is a waste of > cycles. > > Oversight in commit 1de58df4, which added page-level freezing. > > Author: Peter Geoghegan <p...@bowt.ie> > Discussion: > https://postgr.es/m/cah2-wzkzpe4k6qmfet8h4qyjckc2r7tpvksbva7jc9w7igx...@mail.gmail.com
There's some changes from TransactionIdDidCommit() to !TransactionIdDidAbort() that don't look right to me. If the server crashed while xid X was in-progress, TransactionIdDidCommit(X) will return false, but so will TransactionIdDidAbort(X). So besides moving when the check happens you also changed what's being checked in a more substantial way. Also, why did you change when MarkBufferDirty() happens? Previously it happened before we modify the page contents, now after. That's probably fine (it's the order suggested in transam/README), but seems like a mighty subtle thing to change at the same time as something unrelated, particularly without even mentioning it? Greetings, Andres Freund
