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