From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org, Peter Geoghegan <pg(at)bowt(dot)ie> |
Subject: | heap_abort_speculative() sets xmin to Invalid* without HEAP_XMIN_INVALID |
Date: | 2020-07-23 19:40:42 |
Message-ID: | 20200723194042.bygzsr6m23pwwkhd@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-07-23 19:43:44 | Re: Making CASE error handling less surprising |
Previous Message | Andres Freund | 2020-07-23 19:02:18 | Re: Making CASE error handling less surprising |