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

Commitf550833

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 parent1f1eedd commitf550833

File tree

3 files changed

+27
-5
lines changed

3 files changed

+27
-5
lines changed

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

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

358-
/* Set XID to use in redirection tuples */
358+
/*
359+
* Set horizon XID to use in redirection tuples. Use our own XID if we
360+
* have one, else use InvalidTransactionId. The latter case can happen in
361+
* VACUUM or REINDEX CONCURRENTLY, and in neither case would it be okay to
362+
* force an XID to be assigned. VACUUM won't create any redirection
363+
* tuples anyway, but REINDEX CONCURRENTLY can. Fortunately, REINDEX
364+
* CONCURRENTLY doesn't mark the index valid until the end, so there could
365+
* never be any concurrent scans "in flight" to a redirection tuple it has
366+
* inserted. And it locks out VACUUM until the end, too. So it's okay
367+
* for VACUUM to immediately expire a redirection tuple that contains an
368+
* invalid xid.
369+
*/
359370
state->myXid=GetTopTransactionIdIfAny();
360371

361372
/* Assume we're not in an index build (spgbuild will override) */
@@ -1081,7 +1092,6 @@ spgFormDeadTuple(SpGistState *state, int tupstate,
10811092
if (tupstate==SPGIST_REDIRECT)
10821093
{
10831094
ItemPointerSet(&tuple->pointer,blkno,offnum);
1084-
Assert(TransactionIdIsValid(state->myXid));
10851095
tuple->xid=state->myXid;
10861096
}
10871097
else

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

Lines changed: 13 additions & 2 deletions
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
@@ -524,8 +526,17 @@ vacuumRedirectAndPlaceholder(Relation index, Buffer buffer)
524526

525527
dt= (SpGistDeadTuple)PageGetItem(page,PageGetItemId(page,i));
526528

529+
/*
530+
* We can convert a REDIRECT to a PLACEHOLDER if there could no longer
531+
* be any index scans "in flight" to it. Such an index scan would
532+
* have to be in a transaction whose snapshot sees the REDIRECT's XID
533+
* as still running, so comparing the XID against global xmin is a
534+
* conservatively safe test. If the XID is invalid, it must have been
535+
* inserted by REINDEX CONCURRENTLY, so we can zap it immediately.
536+
*/
527537
if (dt->tupstate==SPGIST_REDIRECT&&
528-
GlobalVisTestIsRemovableXid(vistest,dt->xid))
538+
(!TransactionIdIsValid(dt->xid)||
539+
GlobalVisTestIsRemovableXid(vistest,dt->xid)))
529540
{
530541
dt->tupstate=SPGIST_PLACEHOLDER;
531542
Assert(opaque->nRedirection>0);

‎src/include/access/spgist_private.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,8 @@ typedef struct SpGistLeafTupleData
421421
* field, to satisfy some Asserts that we make when replacing a leaf tuple
422422
* with a dead tuple.
423423
* We don't use t_info, but it's needed to align the pointer field.
424-
* pointer and xid are only valid when tupstate = REDIRECT.
424+
* pointer and xid are only valid when tupstate = REDIRECT, and in some
425+
* cases xid can be InvalidTransactionId even then; see initSpGistState.
425426
*/
426427
typedefstructSpGistDeadTupleData
427428
{

0 commit comments

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