Re: Allow ERROR from heap_prepare_freeze_tuple to be downgraded to WARNING

On Tue, Sep 15, 2020 at 2:35 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2020-09-14 17:00:48 -0400, Robert Haas wrote:
> > On Mon, Sep 14, 2020 at 4:13 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > My understanding of the case we're discussing is that it's corruption
> > > (e.g. relfrozenxid being different than table contents) affecting a HOT
> > > chain. I.e. by definition all within a single page. We won't have
> > > modified part of it independent of B < A, because freezing is
> > > all-or-nothing. Just breaking the HOT chain into two or something like
> > > that will just make things worse, because indexes won't find tuples, and
> > > because reindexing might then get confused e.g. by HOT chains without a
> > > valid start, or by having two visible tuples for the same PK.
> >
> > If we adopt the proposal made by Dilip, we will not do that. We must
> > have a.xmax = b.xmin, and that value is either less than relfrozenxid
> > or it is not. If we skip an entire tuple because one XID is bad, then
> > we could break the HOT chain when a.xmin is bad and the remaining
> > values are OK. But if we decide separately for xmin and xmax then we
> > should be alright.
>
> I thought I precisely addressed this case:
>
> > What exactly are you going to put into xmin/xmax here? And how would
> > anything you put into the first tuple not break index lookups? There's
> > no such thing as a frozen xmax (so far), so what are you going to put
> > in there? A random different xid? FrozenTransactionId?
> > HEAP_XMAX_INVALID?
>
> What am I missing?

What problem do you see if we set xmax to the InvalidTransactionId and
HEAP_XMAX_INVALID flag in the infomask ? I mean now also if the xmax
is older than the cutoff xid then we do the same thing i.e.
if (freeze_xmax)
{
..
frz->xmax = InvalidTransactionId;
..
frz->t_infomask &= ~HEAP_XMAX_BITS;
frz->t_infomask |= HEAP_XMAX_INVALID;
frz->t_infomask2 &= ~HEAP_HOT_UPDATED;
frz->t_infomask2 &= ~HEAP_KEYS_UPDATED;
changed = true;
}

So if we do that it will not be part of the hot chain anymore. I
might be missing something but could not see how it can be more broken
than what it is without our change. I agree that in case of corrupted
xmin it can now mark tuple with HEAP_XMAX_INVALID without freezing the
xmin but that is anyway a valid status for a tuple.

However, if we think it still can cause some issues then I feel that
we can skip the whole page as Robert suggested.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com