On 19/03/2024 14:07, Tender Wang wrote: > Thanks for your report. I can reproduce this issue. > I try to delete the Assert, no coredump anymore. > I need some time to learn GiST to find the root cause. At first glance, I think the assertion is too strict. In gistFindCorrectParent(), if we walk right, we update the parent's block number and reset its memorized downlinkoffnum to InvalidOffsetNumber. If we later call gistFindCorrectParent() again with the same stack, because the parent also needs to be split, we hit that assertion. But it's OK in that case, we don't know the downlink because we had moved right. Attached patch relaxes that.
I tested the code with attached patch many times. No failure anymore.
+1
But now I'm having some second thoughts. gistFindCorrectParent() looks like this:
> /* > * 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) > { > OffsetNumber i; > > maxoff = PageGetMaxOffsetNumber(parent->page); > for (i = FirstOffsetNumber; i <= maxoff; i = OffsetNumberNext(i)) > { > iid = PageGetItemId(parent->page, i); > idxtuple = (IndexTuple) PageGetItem(parent->page, iid); > if (ItemPointerGetBlockNumber(&(idxtuple->t_tid)) == child->blkno) > { > /* yes!!, found */ > child->downlinkoffnum = i; > return; > } > } > > parent->blkno = GistPageGetOpaque(parent->page)->rightlink; > parent->downlinkoffnum = InvalidOffsetNumber; > UnlockReleaseBuffer(parent->buffer); > if (parent->blkno == InvalidBlockNumber) > { > /* > * End of chain and still didn't find parent. It's a very-very > * rare situation when the root was split. > */ > break; > } > parent->buffer = ReadBuffer(r, parent->blkno); > LockBuffer(parent->buffer, GIST_EXCLUSIVE); > gistcheckpage(r, parent->buffer); > parent->page = (Page) BufferGetPage(parent->buffer); > }
When we step right and update parent->blkno, shouldn't we also update parent->lsn? Otherwise, we might fail to detect a concurrent page split with the LSN-NSN interlock checks. Or maybe it's correct, and we should indeed not update the memorized LSN. Or maybe it doesn't matter because we retry from the parent anyway, thanks to the 'retry_from_parent' flag? I'm not sure..
I go through the code many times. I found that updating parent->lsn or not
seemd to have no impact. Sorry, I'm not 100% sure.
If you're interested to work on this, Tender, maybe you can figure that out?
