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

Commit507a900

Browse files
committed
Fix insertion of SP-GiST REDIRECT tuples during REINDEX CONCURRENTLY.
Reconstruction of an SP-GiST index by REINDEX CONCURRENTLY mayinsert some REDIRECT tuples. This will typically happen ina transaction that lacks an XID, which leads either to assertionfailure in spgFormDeadTuple or to insertion of a REDIRECT tuplewith zero xid. The latter's not good either, since eventuallyVACUUM will apply GlobalVisTestIsRemovableXid() to the zero xid,resulting in either an assertion failure or a garbage answer.In practice, since REINDEX CONCURRENTLY locks out index scanstill it's done, it doesn't matter whether it inserts REDIRECTsor PLACEHOLDERs; and likewise it doesn't matter how soon VACUUMreduces such a REDIRECT to a PLACEHOLDER. So in non-assert buildsthere's no observable problem here, other than perhaps a littleindex bloat. But it's not behaving as intended.To fix, remove the failing Assert in spgFormDeadTuple, acknowledgingthat we might sometimes insert a zero XID; and guard VACUUM'sGlobalVisTestIsRemovableXid() call with a test for valid XID,ensuring that we'll reduce such a REDIRECT the first time VACUUMsees it. (Versions before v14 use TransactionIdPrecedes here,which won't fail on zero xid, so they really have no bug at allin non-assert builds.)Another solution could be to not create REDIRECTs at all duringREINDEX CONCURRENTLY, making the relevant code paths treat thatcase like index build (which likewise knows that no concurrentindex scans can be happening). That would allow restoring theAssert in spgFormDeadTuple, but we'd still need the VACUUM changebecause redirection tuples with zero xid may be out there already.But there doesn't seem to be a nice way for spginsert() to tell thatit's being called in REINDEX CONCURRENTLY without some API changes,so we'll leave that as a possible future improvement.In HEAD, also rename the SpGistState.myXid field to redirectXid,which seems less misleading (since it might not in fact be ourtransaction's XID) and is certainly less uninformatively generic.Per bug #18499 from Alexander Lakhin. Back-patch to all supportedbranches.Discussion:https://postgr.es/m/18499-8a519c280f956480@postgresql.org
1 parent198de79 commit507a900

File tree

3 files changed

+25
-4
lines changed

3 files changed

+25
-4
lines changed

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,18 @@ initSpGistState(SpGistState *state, Relation index)
206206
/* Make workspace for constructing dead tuples */
207207
state->deadTupleStorage=palloc0(SGDTSIZE);
208208

209-
/* Set XID to use in redirection tuples */
209+
/*
210+
* Set horizon XID to use in redirection tuples. Use our own XID if we
211+
* have one, else use InvalidTransactionId. The latter case can happen in
212+
* VACUUM or REINDEX CONCURRENTLY, and in neither case would it be okay to
213+
* force an XID to be assigned. VACUUM won't create any redirection
214+
* tuples anyway, but REINDEX CONCURRENTLY can. Fortunately, REINDEX
215+
* CONCURRENTLY doesn't mark the index valid until the end, so there could
216+
* never be any concurrent scans "in flight" to a redirection tuple it has
217+
* inserted. And it locks out VACUUM until the end, too. So it's okay
218+
* for VACUUM to immediately expire a redirection tuple that contains an
219+
* invalid xid.
220+
*/
210221
state->myXid=GetTopTransactionIdIfAny();
211222

212223
/* Assume we're not in an index build (spgbuild will override) */
@@ -826,7 +837,6 @@ spgFormDeadTuple(SpGistState *state, int tupstate,
826837
if (tupstate==SPGIST_REDIRECT)
827838
{
828839
ItemPointerSet(&tuple->pointer,blkno,offnum);
829-
Assert(TransactionIdIsValid(state->myXid));
830840
tuple->xid=state->myXid;
831841
}
832842
else

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@ vacuumLeafPage(spgBulkDeleteState *bds, Relation index, Buffer buffer,
189189

190190
/*
191191
* Add target TID to pending list if the redirection could have
192-
* happened since VACUUM started.
192+
* happened since VACUUM started. (If xid is invalid, assume it
193+
* must have happened before VACUUM started, since REINDEX
194+
* CONCURRENTLY locks out VACUUM.)
193195
*
194196
* Note: we could make a tighter test by seeing if the xid is
195197
* "running" according to the active snapshot; but snapmgr.c
@@ -520,6 +522,14 @@ vacuumRedirectAndPlaceholder(Relation index, Buffer buffer)
520522

521523
dt= (SpGistDeadTuple)PageGetItem(page,PageGetItemId(page,i));
522524

525+
/*
526+
* We can convert a REDIRECT to a PLACEHOLDER if there could no longer
527+
* be any index scans "in flight" to it. Such an index scan would
528+
* have to be in a transaction whose snapshot sees the REDIRECT's XID
529+
* as still running, so comparing the XID against global xmin is a
530+
* conservatively safe test. If the XID is invalid, it must have been
531+
* inserted by REINDEX CONCURRENTLY, so we can zap it immediately.
532+
*/
523533
if (dt->tupstate==SPGIST_REDIRECT&&
524534
TransactionIdPrecedes(dt->xid,RecentGlobalXmin))
525535
{

‎src/include/access/spgist_private.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,8 @@ typedef SpGistLeafTupleData *SpGistLeafTuple;
373373
* field, to satisfy some Asserts that we make when replacing a leaf tuple
374374
* with a dead tuple.
375375
* We don't use nextOffset, but it's needed to align the pointer field.
376-
* pointer and xid are only valid when tupstate = REDIRECT.
376+
* pointer and xid are only valid when tupstate = REDIRECT, and in some
377+
* cases xid can be InvalidTransactionId even then; see initSpGistState.
377378
*/
378379
typedefstructSpGistDeadTupleData
379380
{

0 commit comments

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