Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 39eabec

Browse files
nbtree: Move fastpath NULL descent stack assertion.
Commit 074251d added an assertion that verified the fastpath/rightmost page insert optimization's assumption about free space: There should always be enough free space on the page to insert the new item without splitting the page. Otherwise, we end up using the "concurrent root page split" phony/fake stack path in _bt_insert_parent(). This does not lead to incorrect behavior, but it is likely to be far slower than simply using the regular _bt_search() path. The assertion catches serious performance bugs that would probably take a long time to detect any other way. It seems much more natural to make this assertion just before the point that we generate a fake/phony descent stack. Move the assert there. This also makes _bt_insertonpg() a bit more readable.
1 parent c8e8b2f commit 39eabec

File tree

1 file changed

+20
-26
lines changed

1 file changed

+20
-26
lines changed

src/backend/access/nbtree/nbtinsert.c

+20-26
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,13 @@ _bt_doinsert(Relation rel, IndexTuple itup,
174174
/*
175175
* Check if the page is still the rightmost leaf page, has enough
176176
* free space to accommodate the new tuple, and the insertion scan
177-
* key is strictly greater than the first key on the page.
177+
* key is strictly greater than the first key on the page. Note
178+
* that _bt_insert_parent() has an assertion that catches leaf
179+
* page splits that somehow follow from a fastpath insert.
178180
*/
179181
if (P_ISLEAF(lpageop) && P_RIGHTMOST(lpageop) &&
180182
!P_IGNORE(lpageop) &&
181-
(PageGetFreeSpace(page) > insertstate.itemsz) &&
183+
PageGetFreeSpace(page) > insertstate.itemsz &&
182184
PageGetMaxOffsetNumber(page) >= P_FIRSTDATAKEY(lpageop) &&
183185
_bt_compare(rel, itup_key, page, P_FIRSTDATAKEY(lpageop)) > 0)
184186
{
@@ -1140,24 +1142,6 @@ _bt_insertonpg(Relation rel,
11401142
bool is_only = P_LEFTMOST(lpageop) && P_RIGHTMOST(lpageop);
11411143
Buffer rbuf;
11421144

1143-
/*
1144-
* If we're here then a pagesplit is needed. We should never reach
1145-
* here if we're using the fastpath since we should have checked for
1146-
* all the required conditions, including the fact that this page has
1147-
* enough freespace. Note that this routine can in theory deal with
1148-
* the situation where a NULL stack pointer is passed (that's what
1149-
* would happen if the fastpath is taken). But that path is much
1150-
* slower, defeating the very purpose of the optimization. The
1151-
* following assertion should protect us from any future code changes
1152-
* that invalidate those assumptions.
1153-
*
1154-
* Note that whenever we fail to take the fastpath, we clear the
1155-
* cached block. Checking for a valid cached block at this point is
1156-
* enough to decide whether we're in a fastpath or not.
1157-
*/
1158-
Assert(!(P_ISLEAF(lpageop) &&
1159-
BlockNumberIsValid(RelationGetTargetBlock(rel))));
1160-
11611145
/* split the buffer into left and right halves */
11621146
rbuf = _bt_split(rel, itup_key, buf, cbuf, newitemoff, itemsz, itup,
11631147
origitup, nposting, postingoff);
@@ -1370,12 +1354,6 @@ _bt_insertonpg(Relation rel,
13701354
* the optimization for small indexes. We defer that check to this
13711355
* point to ensure that we don't call _bt_getrootheight while holding
13721356
* lock on any other block.
1373-
*
1374-
* We do this after dropping locks on all buffers. So the information
1375-
* about whether the insertion block is still the rightmost block or
1376-
* not may have changed in between. But we will deal with that during
1377-
* next insert operation. No special care is required while setting
1378-
* it.
13791357
*/
13801358
if (BlockNumberIsValid(cachedBlock) &&
13811359
_bt_getrootheight(rel) >= BTREE_FASTPATH_MIN_LEVEL)
@@ -2066,6 +2044,22 @@ _bt_insert_parent(Relation rel,
20662044

20672045
elog(DEBUG2, "concurrent ROOT page split");
20682046
lpageop = (BTPageOpaque) PageGetSpecialPointer(page);
2047+
2048+
/*
2049+
* We should never reach here when a leaf page split takes place
2050+
* despite the insert of newitem being able to apply the fastpath
2051+
* optimization. Make sure of that with an assertion.
2052+
*
2053+
* This is more of a performance issue than a correctness issue.
2054+
* The fastpath won't have a descent stack. Using a phony stack
2055+
* here works, but never rely on that. The fastpath should be
2056+
* rejected when the rightmost leaf page will split, since it's
2057+
* faster to go through _bt_search() and get a stack in the usual
2058+
* way.
2059+
*/
2060+
Assert(!(P_ISLEAF(lpageop) &&
2061+
BlockNumberIsValid(RelationGetTargetBlock(rel))));
2062+
20692063
/* Find the leftmost page at the next level up */
20702064
pbuf = _bt_get_endpoint(rel, lpageop->btpo.level + 1, false,
20712065
NULL);

0 commit comments

Comments
 (0)