Incorrect assumption in heap_prepare_freeze_tuple

From: Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Cc: Andres Freund <andres(at)anarazel(dot)de>
Subject: Incorrect assumption in heap_prepare_freeze_tuple
Date: 2020-10-02 17:56:05
Message-ID: CAGz5QCKOSXnpY4Hk+JfQxEraBHuqnsL6dzozsWi1HwpnAcPYbQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello hackers,

In heap_prepare_freeze_tuple, we make the following assumption:

* It is assumed that the caller has checked the tuple with
* HeapTupleSatisfiesVacuum() and determined that it is not HEAPTUPLE_DEAD
* (else we should be removing the tuple, not freezing it).

Thus, when we see a committed xmax that precedes the cutoff_xid, we throw
the following data corruption error:
errmsg_internal("cannot freeze committed xmax %u", xid)

However, in the caller (lazy_scan_heap), HeapTupleSatisfiesVacuum may
return HEAPTUPLE_DEAD for an updated/deleted tuple that got modified by a
transaction older than OldestXmin. And, if the tuple is HOT-updated, it
should only be removed by a hot-chain prune operation. So, we treat the
tuple as RECENTLY_DEAD and don't remove the tuple.

So, it may lead to an incorrect data corruption error. IIUC, following will
be the exact scenario where the error may happen,

An updated/deleted tuple whose xamx is in between cutoff_xid and
OldestXmin. Since cutoff_xid depends on vacuum_freeze_min_age and
autovacuum_freeze_max_age, it'll not be encountered easily. But, I think
it can be reproduced with some xid burner patch.

I think the fix should be something like following:
if (!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask) &&
- TransactionIdDidCommit(xid))
+ TransactionIdDidCommit(xid) &&
+ !HeapTupleHeaderIsHotUpdated(tuple))
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("cannot freeze committed xmax %u",
xid)));
- freeze_xmax = true;
+
+ freeze_xmax = HeapTupleHeaderIsHotUpdated(tuple) ? false : true;

Attached a patch for the same. Thoughts?

--
Thanks & Regards,
Kuntal Ghosh

Attachment Content-Type Size
0001-Fix-sanity-check-for-HOT-updated-tuple-when-freezing.patch application/octet-stream 2.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-10-02 18:00:38 Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away
Previous Message Mark Wong 2020-10-02 17:45:58 Re: buildfarm animal shoveler failing with "Illegal instruction"