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

Commite43425f

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 parentb793a41 commite43425f

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)
@@ -264,7 +269,7 @@ ginFindParents(GinBtree btree, GinBtreeStack *stack)
264269
ptr->parent=root;
265270
ptr->off=InvalidOffsetNumber;
266271

267-
ginFinishSplit(btree,ptr,false,NULL);
272+
ginFinishOldSplit(btree,ptr,NULL,GIN_EXCLUSIVE);
268273
}
269274

270275
leftmostBlkno=btree->getLeftMostChild(btree,page);
@@ -293,7 +298,7 @@ ginFindParents(GinBtree btree, GinBtreeStack *stack)
293298
ptr->parent=root;
294299
ptr->off=InvalidOffsetNumber;
295300

296-
ginFinishSplit(btree,ptr,false,NULL);
301+
ginFinishOldSplit(btree,ptr,NULL,GIN_EXCLUSIVE);
297302
}
298303
}
299304

@@ -674,15 +679,6 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
674679
booldone;
675680
boolfirst= true;
676681

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

708704
/* move right if it's needed */
709705
page=BufferGetPage(parent->buffer);
@@ -727,7 +723,7 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
727723
page=BufferGetPage(parent->buffer);
728724

729725
if (GinPageIsIncompleteSplit(BufferGetPage(parent->buffer)))
730-
ginFinishSplit(btree,parent,false,buildStats);
726+
ginFinishOldSplit(btree,parent,buildStats,GIN_EXCLUSIVE);
731727
}
732728

733729
/* insert the downlink */
@@ -763,6 +759,42 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
763759
freeGinBtreeStack(stack);
764760
}
765761

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

784816
/* If the leaf page was incompletely split, finish the split first */
785817
if (GinPageIsIncompleteSplit(BufferGetPage(stack->buffer)))
786-
ginFinishSplit(btree,stack,false,buildStats);
818+
ginFinishOldSplit(btree,stack,buildStats,GIN_EXCLUSIVE);
787819

788820
done=ginPlaceToPage(btree,stack,
789821
insertdata,InvalidBlockNumber,

0 commit comments

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