Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite6511fe

Browse files
committed
Fix locking when fixing an incomplete split of a GIN internal page
ginFinishSplit() expects the caller to hold an exclusive lock on thebuffer, but when finishing an earlier "leftover" incomplete split ofan internal page, the caller held a shared lock. That caused anassertion failure in MarkBufferDirty(). Without assertions, it couldlead to corruption if two backends tried to complete the split at thesame time.On master, add a test case using the new injection point facility.Report and analysis by Fei Changhong. Backpatch the fix to allsupported versions.Reviewed-by: Fei Changhong, Michael PaquierDiscussion:https://www.postgresql.org/message-id/tencent_A3CE810F59132D8E230475A5F0F7A08C8307@qq.com
1 parentc3bdb25 commite6511fe

File tree

1 file changed

+50
-18
lines changed

1 file changed

+50
-18
lines changed

‎src/backend/access/gin/ginbtree.c‎

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ static bool ginPlaceToPage(GinBtree btree, GinBtreeStack *stack,
2828
Bufferchildbuf,GinStatsData*buildStats);
2929
staticvoidginFinishSplit(GinBtreebtree,GinBtreeStack*stack,
3030
boolfreestack,GinStatsData*buildStats);
31+
staticvoidginFinishOldSplit(GinBtreebtree,GinBtreeStack*stack,
32+
GinStatsData*buildStats,intaccess);
3133

3234
/*
3335
* Lock buffer by needed method for search.
@@ -109,7 +111,7 @@ ginFindLeafPage(GinBtree btree, bool searchMode,
109111
* encounter on the way.
110112
*/
111113
if (!searchMode&&GinPageIsIncompleteSplit(page))
112-
ginFinishSplit(btree,stack,false,NULL);
114+
ginFinishOldSplit(btree,stack,NULL,access);
113115

114116
/*
115117
* ok, page is correctly locked, we should check to move right ..,
@@ -130,7 +132,7 @@ ginFindLeafPage(GinBtree btree, bool searchMode,
130132
TestForOldSnapshot(snapshot,btree->index,page);
131133

132134
if (!searchMode&&GinPageIsIncompleteSplit(page))
133-
ginFinishSplit(btree,stack,false,NULL);
135+
ginFinishOldSplit(btree,stack,NULL,access);
134136
}
135137

136138
if (GinPageIsLeaf(page))/* we found, return locked page */
@@ -166,8 +168,11 @@ ginFindLeafPage(GinBtree btree, bool searchMode,
166168
* Step right from current page.
167169
*
168170
* The next page is locked first, before releasing the current page. This is
169-
* crucial to protect from concurrent page deletion (see comment in
170-
* ginDeletePage).
171+
* crucial to prevent concurrent VACUUM from deleting a page that we are about
172+
* to step to. (The lock-coupling isn't strictly necessary when we are
173+
* traversing the tree to find an insert location, because page deletion grabs
174+
* a cleanup lock on the root to prevent any concurrent inserts. See Page
175+
* deletion section in the README. But there's no harm in doing it always.)
171176
*/
172177
Buffer
173178
ginStepRight(Bufferbuffer,Relationindex,intlockmode)
@@ -265,7 +270,7 @@ ginFindParents(GinBtree btree, GinBtreeStack *stack)
265270
ptr->parent=root;
266271
ptr->off=InvalidOffsetNumber;
267272

268-
ginFinishSplit(btree,ptr,false,NULL);
273+
ginFinishOldSplit(btree,ptr,NULL,GIN_EXCLUSIVE);
269274
}
270275

271276
leftmostBlkno=btree->getLeftMostChild(btree,page);
@@ -294,7 +299,7 @@ ginFindParents(GinBtree btree, GinBtreeStack *stack)
294299
ptr->parent=root;
295300
ptr->off=InvalidOffsetNumber;
296301

297-
ginFinishSplit(btree,ptr,false,NULL);
302+
ginFinishOldSplit(btree,ptr,NULL,GIN_EXCLUSIVE);
298303
}
299304
}
300305

@@ -676,15 +681,6 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
676681
booldone;
677682
boolfirst= true;
678683

679-
/*
680-
* freestack == false when we encounter an incompletely split page during
681-
* a scan, while freestack == true is used in the normal scenario that a
682-
* split is finished right after the initial insert.
683-
*/
684-
if (!freestack)
685-
elog(DEBUG1,"finishing incomplete split of block %u in gin index \"%s\"",
686-
stack->blkno,RelationGetRelationName(btree->index));
687-
688684
/* this loop crawls up the stack until the insertion is complete */
689685
do
690686
{
@@ -705,7 +701,7 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
705701
* would fail.
706702
*/
707703
if (GinPageIsIncompleteSplit(BufferGetPage(parent->buffer)))
708-
ginFinishSplit(btree,parent,false,buildStats);
704+
ginFinishOldSplit(btree,parent,buildStats,GIN_EXCLUSIVE);
709705

710706
/* move right if it's needed */
711707
page=BufferGetPage(parent->buffer);
@@ -729,7 +725,7 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
729725
page=BufferGetPage(parent->buffer);
730726

731727
if (GinPageIsIncompleteSplit(BufferGetPage(parent->buffer)))
732-
ginFinishSplit(btree,parent,false,buildStats);
728+
ginFinishOldSplit(btree,parent,buildStats,GIN_EXCLUSIVE);
733729
}
734730

735731
/* insert the downlink */
@@ -765,6 +761,42 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
765761
freeGinBtreeStack(stack);
766762
}
767763

764+
/*
765+
* An entry point to ginFinishSplit() that is used when we stumble upon an
766+
* existing incompletely split page in the tree, as opposed to completing a
767+
* split that we just made outselves. The difference is that stack->buffer may
768+
* be merely share-locked on entry, and will be upgraded to exclusive mode.
769+
*
770+
* Note: Upgrading the lock momentarily releases it. Doing that in a scan
771+
* would not be OK, because a concurrent VACUUM might delete the page while
772+
* we're not holding the lock. It's OK in an insert, though, because VACUUM
773+
* has a different mechanism that prevents it from running concurrently with
774+
* inserts. (Namely, it holds a cleanup lock on the root.)
775+
*/
776+
staticvoid
777+
ginFinishOldSplit(GinBtreebtree,GinBtreeStack*stack,GinStatsData*buildStats,intaccess)
778+
{
779+
elog(DEBUG1,"finishing incomplete split of block %u in gin index \"%s\"",
780+
stack->blkno,RelationGetRelationName(btree->index));
781+
782+
if (access==GIN_SHARE)
783+
{
784+
LockBuffer(stack->buffer,GIN_UNLOCK);
785+
LockBuffer(stack->buffer,GIN_EXCLUSIVE);
786+
787+
if (!GinPageIsIncompleteSplit(BufferGetPage(stack->buffer)))
788+
{
789+
/*
790+
* Someone else already completed the split while we were not
791+
* holding the lock.
792+
*/
793+
return;
794+
}
795+
}
796+
797+
ginFinishSplit(btree,stack, false,buildStats);
798+
}
799+
768800
/*
769801
* Insert a value to tree described by stack.
770802
*
@@ -785,7 +817,7 @@ ginInsertValue(GinBtree btree, GinBtreeStack *stack, void *insertdata,
785817

786818
/* If the leaf page was incompletely split, finish the split first */
787819
if (GinPageIsIncompleteSplit(BufferGetPage(stack->buffer)))
788-
ginFinishSplit(btree,stack,false,buildStats);
820+
ginFinishOldSplit(btree,stack,buildStats,GIN_EXCLUSIVE);
789821

790822
done=ginPlaceToPage(btree,stack,
791823
insertdata,InvalidBlockNumber,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp