On 2020-Aug-12, Alvaro Herrera wrote: > 'anyvisible' mode is not required AFAICS; reading the code, I think this > could also hit REINDEX CONCURRENTLY and CREATE INDEX CONCURRENTLY, which > do not use that flag. I didn't try to reproduce it there, though. > Anyway, I'm going to remove that Assert() I added.
So this is what I propose as the final form of the fix. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From e0abe5d957155285980a40fb33c192100699e8c0 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Wed, 12 Aug 2020 14:02:58 -0400 Subject: [PATCH v4] fix HOT tuples while scanning for index builds --- src/backend/access/heap/heapam_handler.c | 20 ++++++++++++++++++++ src/backend/access/heap/pruneheap.c | 5 +++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 267a6ee25a..ba44e30035 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1324,6 +1324,12 @@ heapam_index_build_range_scan(Relation heapRelation, * buffer continuously while visiting the page, so no pruning * operation can occur either. * + * In cases with only ShareUpdateExclusiveLock on the table, it's + * possible for some HOT tuples to appear that we didn't know about + * when we first read the page. To handle that case, we re-obtain the + * list of root offsets when a HOT tuple points to a root item that we + * don't know about. + * * Also, although our opinions about tuple liveness could change while * we scan the page (due to concurrent transaction commits/aborts), * the chain root locations won't, so this info doesn't need to be @@ -1625,6 +1631,20 @@ heapam_index_build_range_scan(Relation heapRelation, offnum = ItemPointerGetOffsetNumber(&heapTuple->t_self); + /* + * If a HOT tuple points to a root that we don't know + * about, obtain root items afresh. If that still fails, + * report it as corruption. + */ + if (root_offsets[offnum - 1] == InvalidOffsetNumber) + { + Page page = BufferGetPage(hscan->rs_cbuf); + + LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_SHARE); + heap_get_root_tuples(page, root_offsets); + LockBuffer(hscan->rs_cbuf, BUFFER_LOCK_UNLOCK); + } + if (!OffsetNumberIsValid(root_offsets[offnum - 1])) ereport(ERROR, (errcode(ERRCODE_DATA_CORRUPTED), diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 256df4de10..7e3d44dfd6 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -732,7 +732,7 @@ heap_page_prune_execute(Buffer buffer, * root_offsets[k - 1] = j. * * The passed-in root_offsets array must have MaxHeapTuplesPerPage entries. - * We zero out all unused entries. + * Unused entries are filled with InvalidOffsetNumber (zero). * * The function must be called with at least share lock on the buffer, to * prevent concurrent prune operations. @@ -747,7 +747,8 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets) OffsetNumber offnum, maxoff; - MemSet(root_offsets, 0, MaxHeapTuplesPerPage * sizeof(OffsetNumber)); + MemSet(root_offsets, InvalidOffsetNumber, + MaxHeapTuplesPerPage * sizeof(OffsetNumber)); maxoff = PageGetMaxOffsetNumber(page); for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum = OffsetNumberNext(offnum)) -- 2.20.1