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

On Tue, Jan 3, 2023 at 7:56 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> I still think these moderation rules are deeply unhelpful...

Yes, it is rather annoying.

> I don't know - I think there's a explicit comment somewhere, but I couldn't
> find it immediately. There's a bunch of indirect references to in in
> heapam_visibility.c, with comments like "it must have aborted or
> crashed".

I think that that's a far cry from any kind of documentation...

> The reason for the behaviour is that we do not have any mechanism for going
> through the clog and aborting all in-progress-during-crash transactions. So
> we'll end up with the clog for all in-progress-during-crash transaction being
> zero / TRANSACTION_STATUS_IN_PROGRESS.

I find this astonishing. Why isn't there a prominent comment that
advertises that TransactionIdDidAbort() just doesn't work reliably?

> IMO it's almost always wrong to use TransactionIdDidAbort().

I didn't think that there was any general guarantee about
TransactionIdDidAbort() working after a crash. But this is an on-disk
XID, taken from some tuple's xmax, which must have a value <
OldestXmin.

> I think changes in how WAL logging is done are just about always worth
> mentioning in a commit message...

I agree with that as a general statement, but I never imagined that
this was a case that such a statement could apply to.

I will try to remember to put something about similar changes in any
future commit messages, in the unlikely event that I ever end up
moving MarkBufferDirty() around in some existing critical section in
the future.

--
Peter Geoghegan