Re: pgsql: Delay commit status checks until freezing executes.

On Tue, Jan 3, 2023 at 4:54 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> 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.

I did point this out on the thread. I made this change with the
intention of making the check more robust. Apparently this was
misguided.

Where is the behavior that you describe documented, if anywhere?

> 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?

I changed it because the new order is idiomatic. I didn't think that
this was particularly worth mentioning, or even subtle. The logic from
heap_execute_freeze_tuple() only performs simple in-place
modifications.

--
Peter Geoghegan