Commit 67b0b2db authored by Peter Geoghegan's avatar Peter Geoghegan

Reconsider nbtree page deletion assertion.

Commit 624686ab added an assertion that verified that _bt_search
successfully relocated the leaf page undergoing deletion.  Page deletion
cannot deal with the case where the descent stack is to the right of the
page, so this seemed critical (deletion can only handle the case where
the descent stack is to the left of the leaf/target page).  However, the
assertion went a bit too far.

Since only a buffer pin is held on the leaf page throughout the call to
_bt_search, nothing guarantees that it can't have split during this
small window.  And if does actually split, _bt_search may end up
"relocating" a page to the right of the original target leaf page.  This
scenario seems extremely unlikely, but it must still be considered.
Remove the assertion, and document how we cope in this scenario.
parent c301c2e7
...@@ -1599,8 +1599,6 @@ _bt_pagedel(Relation rel, Buffer leafbuf, TransactionId *oldestBtpoXact) ...@@ -1599,8 +1599,6 @@ _bt_pagedel(Relation rel, Buffer leafbuf, TransactionId *oldestBtpoXact)
itup_key->pivotsearch = true; itup_key->pivotsearch = true;
stack = _bt_search(rel, itup_key, &sleafbuf, BT_READ, NULL); stack = _bt_search(rel, itup_key, &sleafbuf, BT_READ, NULL);
/* won't need a second lock or pin on leafbuf */ /* won't need a second lock or pin on leafbuf */
Assert(leafblkno == BufferGetBlockNumber(sleafbuf) ||
!itup_key->heapkeyspace);
_bt_relbuf(rel, sleafbuf); _bt_relbuf(rel, sleafbuf);
/* /*
...@@ -1608,6 +1606,15 @@ _bt_pagedel(Relation rel, Buffer leafbuf, TransactionId *oldestBtpoXact) ...@@ -1608,6 +1606,15 @@ _bt_pagedel(Relation rel, Buffer leafbuf, TransactionId *oldestBtpoXact)
* within _bt_mark_page_halfdead. We must do it that way * within _bt_mark_page_halfdead. We must do it that way
* because it's possible that leafbuf can no longer be * because it's possible that leafbuf can no longer be
* deleted. We need to recheck. * deleted. We need to recheck.
*
* Note: We can't simply hold on to the sleafbuf lock instead,
* because it's barely possible that sleafbuf is not the same
* page as leafbuf. This happens when leafbuf split after our
* original lock was dropped, but before _bt_search finished
* its descent. We rely on the assumption that we'll find
* leafbuf isn't safe to delete anymore in this scenario.
* (Page deletion can cope with the stack being to the left of
* leafbuf, but not to the right of leafbuf.)
*/ */
LockBuffer(leafbuf, BT_WRITE); LockBuffer(leafbuf, BT_WRITE);
continue; continue;
...@@ -1735,9 +1742,8 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack) ...@@ -1735,9 +1742,8 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack)
* Before attempting to lock the parent page, check that the right sibling * Before attempting to lock the parent page, check that the right sibling
* is not in half-dead state. A half-dead right sibling would have no * is not in half-dead state. A half-dead right sibling would have no
* downlink in the parent, which would be highly confusing later when we * downlink in the parent, which would be highly confusing later when we
* delete the downlink that follows the leafbuf page's downlink. It would * delete the downlink. It would fail the "right sibling of target page
* fail the "right sibling of target page is also the next child in parent * is also the next child in parent page" cross-check below.
* page" cross-check below.
*/ */
if (_bt_rightsib_halfdeadflag(rel, leafrightsib)) if (_bt_rightsib_halfdeadflag(rel, leafrightsib))
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment