Re: HOT chain validation in verify_heapam()

From: Andres Freund <andres(at)anarazel(dot)de>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Aleksander Alekseev <aleksander(at)timescale(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Himanshu Upadhyaya <upadhyaya(dot)himanshu(at)gmail(dot)com>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
Subject: Re: HOT chain validation in verify_heapam()
Date: 2023-03-22 20:45:52
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2023-03-22 09:19:18 -0400, Robert Haas wrote:
> On Fri, Mar 17, 2023 at 8:31 AM Aleksander Alekseev
> <aleksander(at)timescale(dot)com> wrote:
> > The patch needed a rebase due to a4f23f9b. PFA v12.
> I have committed this after tidying up a bunch of things in the test
> case file that I found too difficult to understand -- or in some cases
> just incorrect, like:

As noticed by Andrew and
then reproduced on HEAD by me, there's something not right here.

At the very least there's missing verification that tids actually exists in the
"Update chain validation" loop, leading to:
TRAP: failed Assert("ItemIdHasStorage(itemId)"), File: "../../../../home/andres/src/postgresql/src/include/storage/bufpage.h", Line: 355, PID: 645093

Which makes sense - the earlier loop adds t_ctid to the successor array, which
we then query without checking if there still is such a tid on the page.

I suspect we don't just need a !ItemIdIsUsed(), but also a check gainst the
max offset on the page.

WRT these failures:
non-heap-only update produced a heap-only tuple at offset 20

I think that's largely a consequence of HeapTupleHeaderIsHotUpdated()'s
* Note that we stop considering a tuple HOT-updated as soon as it is known
* aborted or the would-be updating transaction is known aborted. For best
* efficiency, check tuple visibility before using this macro, so that the
* INVALID bits will be as up to date as possible.
#define HeapTupleHeaderIsHotUpdated(tup) \
( \
((tup)->t_infomask2 & HEAP_HOT_UPDATED) != 0 && \
((tup)->t_infomask & HEAP_XMAX_INVALID) == 0 && \
!HeapTupleHeaderXminInvalid(tup) \

Currently the new verify_heapam() follows ctid chains when XMAX_INVALID is set
and expects to find an item it can dereference - but I don't think that's
something we can rely on: Afaics HOT pruning can break chains, but doesn't
reset xmax.


Andres Freund

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2023-03-22 21:14:38 Re: HOT chain validation in verify_heapam()
Previous Message Tom Lane 2023-03-22 20:29:16 Re: Set arbitrary GUC options during initdb