diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index 994f5f2cb4..773f4ecdb0 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -206,7 +206,18 @@ initSpGistState(SpGistState *state, Relation index) /* Make workspace for constructing dead tuples */ state->deadTupleStorage = palloc0(SGDTSIZE); - /* Set XID to use in redirection tuples */ + /* + * Set horizon XID to use in redirection tuples. Use our own XID if we + * have one, else use InvalidTransactionId. The latter case can happen in + * VACUUM or REINDEX CONCURRENTLY, and in neither case would it be okay to + * force an XID to be assigned. VACUUM won't create any redirection + * tuples anyway, but REINDEX CONCURRENTLY can. Fortunately, REINDEX + * CONCURRENTLY doesn't mark the index valid until the end, so there could + * never be any concurrent scans "in flight" to a redirection tuple it has + * inserted. And it locks out VACUUM until the end, too. So it's okay + * for VACUUM to immediately expire a redirection tuple that contains an + * invalid xid. + */ state->myXid = GetTopTransactionIdIfAny(); /* Assume we're not in an index build (spgbuild will override) */ @@ -826,7 +837,6 @@ spgFormDeadTuple(SpGistState *state, int tupstate, if (tupstate == SPGIST_REDIRECT) { ItemPointerSet(&tuple->pointer, blkno, offnum); - Assert(TransactionIdIsValid(state->myXid)); tuple->xid = state->myXid; } else diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c index bd98707f3c..50adf35d4f 100644 --- a/src/backend/access/spgist/spgvacuum.c +++ b/src/backend/access/spgist/spgvacuum.c @@ -189,7 +189,9 @@ vacuumLeafPage(spgBulkDeleteState *bds, Relation index, Buffer buffer, /* * Add target TID to pending list if the redirection could have - * happened since VACUUM started. + * happened since VACUUM started. (If xid is invalid, assume it + * must have happened before VACUUM started, since REINDEX + * CONCURRENTLY locks out VACUUM.) * * Note: we could make a tighter test by seeing if the xid is * "running" according to the active snapshot; but snapmgr.c @@ -520,6 +522,14 @@ vacuumRedirectAndPlaceholder(Relation index, Buffer buffer) dt = (SpGistDeadTuple) PageGetItem(page, PageGetItemId(page, i)); + /* + * We can convert a REDIRECT to a PLACEHOLDER if there could no longer + * be any index scans "in flight" to it. Such an index scan would + * have to be in a transaction whose snapshot sees the REDIRECT's XID + * as still running, so comparing the XID against global xmin is a + * conservatively safe test. If the XID is invalid, it must have been + * inserted by REINDEX CONCURRENTLY, so we can zap it immediately. + */ if (dt->tupstate == SPGIST_REDIRECT && TransactionIdPrecedes(dt->xid, RecentGlobalXmin)) { diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h index 00b98ec6a0..cd7790ddcb 100644 --- a/src/include/access/spgist_private.h +++ b/src/include/access/spgist_private.h @@ -373,7 +373,8 @@ typedef SpGistLeafTupleData *SpGistLeafTuple; * field, to satisfy some Asserts that we make when replacing a leaf tuple * with a dead tuple. * We don't use nextOffset, but it's needed to align the pointer field. - * pointer and xid are only valid when tupstate = REDIRECT. + * pointer and xid are only valid when tupstate = REDIRECT, and in some + * cases xid can be InvalidTransactionId even then; see initSpGistState. */ typedef struct SpGistDeadTupleData {