Hi,
After adding a few assertions to validate the connection scalability
patch I saw failures that also apply to master:
I added an assertion to TransactionIdIsCurrentTransactionId(),
*IsInProgress(), ... ensuring that the xid is within an expected
range. Which promptly failed in isolation tests.
The reason for that is that heap_abort_speculative() sets xmin to
InvalidTransactionId but does *not* add HEAP_XMIN_INVALID to infomask.
The various HeapTupleSatisfies* routines avoid calling those routines
for invalid xmins by checking HeapTupleHeaderXminInvalid() first. But
since heap_abort_speculative() didn't set that, we end up calling
TransactionIdIsCurrentTransactionId(InvalidTransactionId) which then
triggered my assertion.
Obviously I can trivially fix that by adjusting the assertions to allow
InvalidTransactionId. But to me it seems fragile to only have xmin == 0
in one rare occasion, and to rely on TransactionIdIs* to return
precisely the right thing without those functions necessarily having
been designed with that in mind.
I think we should change heap_abort_speculative() to set
HEAP_XMIN_INVALID in master. But we can't really do anything about
existing tuples without it - therefore we will have to forever take care
about encountering that combination :(.
Perhaps we should instead or additionally make
HeapTupleHeaderXminInvalid() explicitly check for InvalidTransactionId?
Greetings,
Andres Freund
