From 661bd1bbb029f71e0dbe7d37a731cae50afb862b Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 24 Sep 2023 00:49:08 +0300 Subject: [PATCH 4/5] Treat 'downlinkoffnum' only as a hint in gistFindCorrectParent. We have had a number of bugs in trying to meticulously clear 'downlinkoffnum' when we update a parent page. But instead of trying hard to clear it all the right places, we can treat is merely as a hint, so that if we forget to clear it, it still works. This makes the fixes in commit XXX and a7ee7c8513 unnecessary. It doesn't hurt to still clear 'downlinkoffnum', but everything would work without it now. --- src/backend/access/gist/gist.c | 69 ++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index b3749bbb432..42f9408becb 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -1018,34 +1018,46 @@ gistFindPath(Relation r, BlockNumber child, OffsetNumber *downlinkoffnum) * remain so at exit, but it might not be the same page anymore. */ static void -gistFindCorrectParent(Relation r, GISTInsertStack *child) +gistFindCorrectParent(Relation r, GISTInsertStack *child, bool is_build) { GISTInsertStack *parent = child->parent; - -#ifdef USE_ASSERT_CHECKING - int save_downlinkoffnum = child->downlinkoffnum; - BlockNumber save_parent_blkno = parent->blkno; - - child->downlinkoffnum = InvalidOffsetNumber; -#endif + ItemId iid; + IndexTuple idxtuple; + OffsetNumber i, + maxoff; + GISTInsertStack *ptr; gistcheckpage(r, parent->buffer); parent->page = (Page) BufferGetPage(parent->buffer); + maxoff = PageGetMaxOffsetNumber(parent->page); - /* here we don't need to distinguish between split and page update */ - if (child->downlinkoffnum == InvalidOffsetNumber || - parent->lsn != PageGetLSN(parent->page)) + /* Check if the downlink is still where it was before */ + if (child->downlinkoffnum != InvalidOffsetNumber && child->downlinkoffnum <= maxoff) { - /* parent is changed, look child in right links until found */ - OffsetNumber i, - maxoff; - ItemId iid; - IndexTuple idxtuple; - GISTInsertStack *ptr; + iid = PageGetItemId(parent->page, child->downlinkoffnum); + idxtuple = (IndexTuple) PageGetItem(parent->page, iid); + if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno) + return; /* still there */ + } + /* + * The page has changed since we looked. During normal operation, every + * update of a page changes its LSN, so the LSN we memorized should have + * changed too. During index build, however, we don't WAL-log the changes + * until we have built the index, so the LSN doesn't change. There is no + * concurrent activity during index build, but we might have changed the + * parent ourselves. + */ + Assert(parent->lsn != PageGetLSN(parent->page) || is_build); + + /* + * Scan the page to re-find the downlink. If the page was split, it might + * have moved to a different page, so follow the right links until we find + * it. + */ + { while (true) { - maxoff = PageGetMaxOffsetNumber(parent->page); for (i = FirstOffsetNumber; i <= maxoff; i = OffsetNumberNext(i)) { iid = PageGetItemId(parent->page, i); @@ -1054,15 +1066,6 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child) { /* yes!!, found */ child->downlinkoffnum = i; -#ifdef USE_ASSERT_CHECKING - if (parent->lsn == PageGetLSN(parent->page) && save_downlinkoffnum != InvalidOffsetNumber) { - if (i != save_downlinkoffnum) { - elog(PANIC, "expected to find downlink for %u on blk %u offset %u, but it was found on blk %u offset %u", - child->blkno, save_parent_blkno, save_downlinkoffnum, - parent->blkno, i); - } - } -#endif return; } } @@ -1114,7 +1117,7 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child) /* make recursive call to normal processing */ LockBuffer(child->parent->buffer, GIST_EXCLUSIVE); - gistFindCorrectParent(r, child); + gistFindCorrectParent(r, child, is_build); } } @@ -1123,7 +1126,7 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child) */ static IndexTuple gistformdownlink(Relation rel, Buffer buf, GISTSTATE *giststate, - GISTInsertStack *stack) + GISTInsertStack *stack, bool is_build) { Page page = BufferGetPage(buf); OffsetNumber maxoff; @@ -1164,7 +1167,7 @@ gistformdownlink(Relation rel, Buffer buf, GISTSTATE *giststate, ItemId iid; LockBuffer(stack->parent->buffer, GIST_EXCLUSIVE); - gistFindCorrectParent(rel, stack); + gistFindCorrectParent(rel, stack, is_build); iid = PageGetItemId(stack->parent->page, stack->downlinkoffnum); downlink = (IndexTuple) PageGetItem(stack->parent->page, iid); downlink = CopyIndexTuple(downlink); @@ -1210,7 +1213,7 @@ gistfixsplit(GISTInsertState *state, GISTSTATE *giststate) page = BufferGetPage(buf); /* Form the new downlink tuples to insert to parent */ - downlink = gistformdownlink(state->r, buf, giststate, stack); + downlink = gistformdownlink(state->r, buf, giststate, stack, state->is_build); si->buf = buf; si->downlink = downlink; @@ -1367,7 +1370,7 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, right = (GISTPageSplitInfo *) list_nth(splitinfo, pos); left = (GISTPageSplitInfo *) list_nth(splitinfo, pos - 1); - gistFindCorrectParent(state->r, stack); + gistFindCorrectParent(state->r, stack, state->is_build); if (gistinserttuples(state, stack->parent, giststate, &right->downlink, 1, InvalidOffsetNumber, @@ -1393,7 +1396,7 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, */ tuples[0] = left->downlink; tuples[1] = right->downlink; - gistFindCorrectParent(state->r, stack); + gistFindCorrectParent(state->r, stack, state->is_build); (void) gistinserttuples(state, stack->parent, giststate, tuples, 2, stack->downlinkoffnum, -- 2.39.2