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

Commitf120c08

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 parent6b77048 commitf120c08

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

@@ -675,15 +680,6 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
675680
booldone;
676681
boolfirst= true;
677682

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

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

730726
if (GinPageIsIncompleteSplit(BufferGetPage(parent->buffer)))
731-
ginFinishSplit(btree,parent,false,buildStats);
727+
ginFinishOldSplit(btree,parent,buildStats,GIN_EXCLUSIVE);
732728
}
733729

734730
/* insert the downlink */
@@ -764,6 +760,42 @@ ginFinishSplit(GinBtree btree, GinBtreeStack *stack, bool freestack,
764760
freeGinBtreeStack(stack);
765761
}
766762

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

785817
/* If the leaf page was incompletely split, finish the split first */
786818
if (GinPageIsIncompleteSplit(BufferGetPage(stack->buffer)))
787-
ginFinishSplit(btree,stack,false,buildStats);
819+
ginFinishOldSplit(btree,stack,buildStats,GIN_EXCLUSIVE);
788820

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

0 commit comments

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