From 77c8e9594f153291a04b86ec0df2daebc8718765 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 22 Nov 2021 09:52:45 -0800 Subject: [PATCH v4 2/6] Verify redirect items are still correct after heap_page_prune(). Author: Andres Freund Discussion: https://postgr.es/m/20211122175914.ayk6gg6nvdwuhrzb@alap3.anarazel.de --- src/backend/access/heap/pruneheap.c | 57 +++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index baf33c9b988..6557d710262 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -89,6 +89,7 @@ static void heap_prune_record_redirect(PruneState *prstate, OffsetNumber offnum, OffsetNumber rdoffnum); static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum); static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum); +static void page_verify_redirects(Page page); /* @@ -961,6 +962,10 @@ heap_page_prune_execute(Buffer buffer, * indexes when an entire HOT chain becomes dead. A heap-only tuple * can never become LP_DEAD; an LP_REDIRECT item or a regular heap * tuple can. + * + * This check may miss problems, e.g. the target of a redirect could + * be marked as unused subsequently. The page_verify_redirects() check + * below will catch such problems. */ tolp = PageGetItemId(page, tooff); Assert(ItemIdHasStorage(tolp) && ItemIdIsNormal(tolp)); @@ -1030,6 +1035,58 @@ heap_page_prune_execute(Buffer buffer, * whether it has free pointers. */ PageRepairFragmentation(page); + + /* + * Now that the page has been modified, assert that redirect items still + * point to valid targets. + */ + page_verify_redirects(page); +} + + +/* + * If built with assertions, verify that all LP_REDIRECT items point to a + * valid item. + * + * One way that bugs around HOT pruning show is redirect items pointing to + * removed tuples. It's not trivial to reliably check that making an item + * unused will not orphan a redirect item during heap_prune_chain() / + * heap_page_prune_execute(), so we additionally check the whole page after + * pruning. Without this check such bugs would typically only cause asserts + * later, potentially well after the corruption has been introduced. + * + * Also check comments in heap_page_prune_execute()'s redirection loop. + */ +static void +page_verify_redirects(Page page) +{ +#ifdef USE_ASSERT_CHECKING + OffsetNumber offnum; + OffsetNumber maxoff; + + maxoff = PageGetMaxOffsetNumber(page); + for (offnum = FirstOffsetNumber; + offnum <= maxoff; + offnum = OffsetNumberNext(offnum)) + { + ItemId itemid = PageGetItemId(page, offnum); + OffsetNumber targoff; + ItemId targitem; + HeapTupleHeader htup; + + if (!ItemIdIsRedirected(itemid)) + continue; + + targoff = ItemIdGetRedirect(itemid); + targitem = PageGetItemId(page, targoff); + + Assert(ItemIdIsUsed(targitem)); + Assert(ItemIdIsNormal(targitem)); + Assert(ItemIdHasStorage(targitem)); + htup = (HeapTupleHeader) PageGetItem(page, targitem); + Assert(HeapTupleHeaderIsHeapOnly(htup)); + } +#endif } -- 2.34.0