Re: HOT chain validation in verify_heapam() - Mailing list pgsql-hackers

From Peter Geoghegan
Subject Re: HOT chain validation in verify_heapam()
Date
Msg-id CAH2-Wzkh3DMCDRPfhZxj9xCq9v3WmzvmbiCpf1dNKUBPadhCbQ@mail.gmail.com
Whole thread Raw
In response to Re: HOT chain validation in verify_heapam()  (Andres Freund <andres@anarazel.de>)
Responses Re: HOT chain validation in verify_heapam()
List pgsql-hackers
On Tue, Nov 15, 2022 at 10:55 PM Andres Freund <andres@anarazel.de> wrote:
> I'm quite certain that it's possible to end up freezing an earlier row
> versions in a hot chain in < 14, I got there with careful gdb
> orchestration. Of course possible I screwed something up, given I did it once,
> interactively. Not sure if trying to fix it is worth the risk of backpatching
> all the necessary changes to switch to the retry approach.

There is code in heap_prepare_freeze_tuple() that treats a raw xmax as
"xmax_already_frozen = true", even when the raw xmax value isn't
already set to InvalidTransactionId. I'm referring to this code:

    if ( ... ) // process raw xmax
    ....
    else if (TransactionIdIsNormal(xid))
    ....
    else if ((tuple->t_infomask & HEAP_XMAX_INVALID) ||
             !TransactionIdIsValid(HeapTupleHeaderGetRawXmax(tuple)))
    {
        freeze_xmax = false;
        xmax_already_frozen = true;
        /* No need for relfrozenxid_out handling for already-frozen xmax */
    }
    else
        ereport(ERROR,
                (errcode(ERRCODE_DATA_CORRUPTED),
                 errmsg_internal("found xmax %u (infomask 0x%04x) not
frozen, not multi, not normal",
                                 xid, tuple->t_infomask)));

Why should it be okay to not process xmax during this call (by setting
"xmax_already_frozen = true"), just because HEAP_XMAX_INVALID happens
to be set? Isn't HEAP_XMAX_INVALID purely a hint? (HEAP_XMIN_FROZEN is
*not* a hint, but we're dealing with xmax here.)

I'm not sure how relevant this is to the concerns you have about
frozen xmax, or even if it's any kind of problem, but it still seems
worth fixing. It seems to me that there should be clear rules on what
special transaction IDs can appear in xmax. Namely: the only special
transaction ID that can ever appear in xmax is InvalidTransactionId.
(Also, it's not okay to see *any* other XID in the
"xmax_already_frozen = true" path, nor would it be okay to leave any
other XID behind in xmax in the nearby "freeze_xmax = true" path.)

-- 
Peter Geoghegan



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: heavily contended lwlocks with long wait queues scale badly
Next
From: Simon Riggs
Date:
Subject: Re: Reducing power consumption on idle servers