diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/access/spgist/spgutils.c | 14 | ||||
-rw-r--r-- | src/backend/access/spgist/spgvacuum.c | 15 | ||||
-rw-r--r-- | src/include/access/spgist_private.h | 3 |
3 files changed, 27 insertions, 5 deletions
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index 26499166e7c..5fa9e230c08 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -356,7 +356,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) */ @@ -1073,7 +1084,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 8a5b540c809..85bc02b92c8 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 @@ -524,8 +526,17 @@ vacuumRedirectAndPlaceholder(Relation index, Relation heaprel, 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 && - GlobalVisTestIsRemovableXid(vistest, dt->xid)) + (!TransactionIdIsValid(dt->xid) || + GlobalVisTestIsRemovableXid(vistest, dt->xid))) { dt->tupstate = SPGIST_PLACEHOLDER; Assert(opaque->nRedirection > 0); diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h index c6ef46fc206..0af801b75ba 100644 --- a/src/include/access/spgist_private.h +++ b/src/include/access/spgist_private.h @@ -421,7 +421,8 @@ typedef struct SpGistLeafTupleData * field, to satisfy some Asserts that we make when replacing a leaf tuple * with a dead tuple. * We don't use t_info, 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 { |