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

Commitc1b72bf

Browse files
committed
Fix query-cancel handling in spgdoinsert().
Knowing that a buggy opclass could cause an infinite insertion loop,spgdoinsert() intended to allow its loop to be interrupted by querycancel. However, that never actually worked, because in iterationsafter the first, we'd be holding buffer lock(s) which would causeInterruptHoldoffCount to be positive, preventing servicing of theinterrupt.To fix, check if an interrupt is pending, and if so fall out ofthe insertion loop and service the interrupt after we've releasedthe buffers. If it was indeed a query cancel, that's the end ofthe matter. If it was a non-canceling interrupt reason, make useof the existing provision to retry the whole insertion. (This isn'tas wasteful as it might seem, since any upper-level index tuples wealready created should be usable in the next attempt.)While there's no known instance of such a bug in existing releasebranches, it still seems like a good idea to back-patch this toall supported branches, since the behavior is fairly nasty if aloop does happen --- not only is it uncancelable, but it willquickly consume memory to the point of an OOM failure. In anycase, this code is certainly not working as intended.Per report from Dilip Kumar.Discussion:https://postgr.es/m/CAFiTN-uxP_soPhVG840tRMQTBmtA_f_Y8N51G7DKYYqDh7XN-A@mail.gmail.com
1 parent63831c1 commitc1b72bf

File tree

1 file changed

+45
-7
lines changed

1 file changed

+45
-7
lines changed

‎src/backend/access/spgist/spgdoinsert.c

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1884,13 +1884,14 @@ spgSplitNodeAction(Relation index, SpGistState *state,
18841884
* Insert one item into the index.
18851885
*
18861886
* Returns true on success, false if we failed to complete the insertion
1887-
* because of conflict with a concurrent insert. In the latter case,
1888-
* caller should re-call spgdoinsert() with the same args.
1887+
*(typicallybecause of conflict with a concurrent insert). In the latter
1888+
*case,caller should re-call spgdoinsert() with the same args.
18891889
*/
18901890
bool
18911891
spgdoinsert(Relationindex,SpGistState*state,
18921892
ItemPointerheapPtr,Datumdatum,boolisnull)
18931893
{
1894+
boolresult= true;
18941895
intlevel=0;
18951896
DatumleafDatum;
18961897
intleafSize;
@@ -1974,16 +1975,33 @@ spgdoinsert(Relation index, SpGistState *state,
19741975
parent.offnum=InvalidOffsetNumber;
19751976
parent.node=-1;
19761977

1978+
/*
1979+
* Before entering the loop, try to clear any pending interrupt condition.
1980+
* If a query cancel is pending, we might as well accept it now not later;
1981+
* while if a non-canceling condition is pending, servicing it here avoids
1982+
* having to restart the insertion and redo all the work so far.
1983+
*/
1984+
CHECK_FOR_INTERRUPTS();
1985+
19771986
for (;;)
19781987
{
19791988
boolisNew= false;
19801989

19811990
/*
19821991
* Bail out if query cancel is pending. We must have this somewhere
19831992
* in the loop since a broken opclass could produce an infinite
1984-
* picksplit loop.
1993+
* picksplit loop. However, because we'll be holding buffer lock(s)
1994+
* after the first iteration, ProcessInterrupts() wouldn't be able to
1995+
* throw a cancel error here. Hence, if we see that an interrupt is
1996+
* pending, break out of the loop and deal with the situation below.
1997+
* Set result = false because we must restart the insertion if the
1998+
* interrupt isn't a query-cancel-or-die case.
19851999
*/
1986-
CHECK_FOR_INTERRUPTS();
2000+
if (INTERRUPTS_PENDING_CONDITION())
2001+
{
2002+
result= false;
2003+
break;
2004+
}
19872005

19882006
if (current.blkno==InvalidBlockNumber)
19892007
{
@@ -2102,10 +2120,14 @@ spgdoinsert(Relation index, SpGistState *state,
21022120
* spgAddNode and spgSplitTuple cases will loop back to here to
21032121
* complete the insertion operation. Just in case the choose
21042122
* function is broken and produces add or split requests
2105-
* repeatedly, check for query cancel.
2123+
* repeatedly, check for query cancel (see comments above).
21062124
*/
21072125
process_inner_tuple:
2108-
CHECK_FOR_INTERRUPTS();
2126+
if (INTERRUPTS_PENDING_CONDITION())
2127+
{
2128+
result= false;
2129+
break;
2130+
}
21092131

21102132
innerTuple= (SpGistInnerTuple)PageGetItem(current.page,
21112133
PageGetItemId(current.page,current.offnum));
@@ -2228,5 +2250,21 @@ spgdoinsert(Relation index, SpGistState *state,
22282250
UnlockReleaseBuffer(parent.buffer);
22292251
}
22302252

2231-
return true;
2253+
/*
2254+
* We do not support being called while some outer function is holding a
2255+
* buffer lock (or any other reason to postpone query cancels). If that
2256+
* were the case, telling the caller to retry would create an infinite
2257+
* loop.
2258+
*/
2259+
Assert(INTERRUPTS_CAN_BE_PROCESSED());
2260+
2261+
/*
2262+
* Finally, check for interrupts again. If there was a query cancel,
2263+
* ProcessInterrupts() will be able to throw the error here. If it was
2264+
* some other kind of interrupt that can just be cleared, return false to
2265+
* tell our caller to retry.
2266+
*/
2267+
CHECK_FOR_INTERRUPTS();
2268+
2269+
returnresult;
22322270
}

0 commit comments

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