On Tue, Jan 3, 2023 at 7:56 PM Andres Freund <andres@anarazel.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