diff options
| -rw-r--r-- | src/backend/access/spgist/spgutils.c | 18 | ||||
| -rw-r--r-- | src/backend/access/spgist/spgvacuum.c | 15 | ||||
| -rw-r--r-- | src/backend/access/spgist/spgxlog.c | 2 | ||||
| -rw-r--r-- | src/include/access/spgist_private.h | 7 | ||||
| -rw-r--r-- | src/include/access/spgxlog.h | 2 |
5 files changed, 33 insertions, 11 deletions
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index 3f793125f74..76b80146ff0 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -358,8 +358,19 @@ initSpGistState(SpGistState *state, Relation index) /* Make workspace for constructing dead tuples */ state->deadTupleStorage = palloc0(SGDTSIZE); - /* Set XID to use in redirection tuples */ - state->myXid = GetTopTransactionIdIfAny(); + /* + * 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->redirectXid = GetTopTransactionIdIfAny(); /* Assume we're not in an index build (spgbuild will override) */ state->isBuild = false; @@ -1075,8 +1086,7 @@ spgFormDeadTuple(SpGistState *state, int tupstate, if (tupstate == SPGIST_REDIRECT) { ItemPointerSet(&tuple->pointer, blkno, offnum); - Assert(TransactionIdIsValid(state->myXid)); - tuple->xid = state->myXid; + tuple->xid = state->redirectXid; } else { diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c index d2e1624924a..0da069fd4d7 100644 --- a/src/backend/access/spgist/spgvacuum.c +++ b/src/backend/access/spgist/spgvacuum.c @@ -188,7 +188,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 @@ -523,8 +525,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/backend/access/spgist/spgxlog.c b/src/backend/access/spgist/spgxlog.c index 11d006998ed..4e31d33e709 100644 --- a/src/backend/access/spgist/spgxlog.c +++ b/src/backend/access/spgist/spgxlog.c @@ -36,7 +36,7 @@ fillFakeState(SpGistState *state, spgxlogState stateSrc) { memset(state, 0, sizeof(*state)); - state->myXid = stateSrc.myXid; + state->redirectXid = stateSrc.redirectXid; state->isBuild = stateSrc.isBuild; state->deadTupleStorage = palloc0(SGDTSIZE); } diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h index 2e9c757b302..e7cbe10a89b 100644 --- a/src/include/access/spgist_private.h +++ b/src/include/access/spgist_private.h @@ -157,7 +157,7 @@ typedef struct SpGistState char *deadTupleStorage; /* workspace for spgFormDeadTuple */ - TransactionId myXid; /* XID to use when creating a redirect tuple */ + TransactionId redirectXid; /* XID to use when creating a redirect tuple */ bool isBuild; /* true if doing index build */ } SpGistState; @@ -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 { @@ -464,7 +465,7 @@ typedef SpGistDeadTupleData *SpGistDeadTuple; #define STORE_STATE(s, d) \ do { \ - (d).myXid = (s)->myXid; \ + (d).redirectXid = (s)->redirectXid; \ (d).isBuild = (s)->isBuild; \ } while(0) diff --git a/src/include/access/spgxlog.h b/src/include/access/spgxlog.h index a08f0dc8d5c..16cc7350012 100644 --- a/src/include/access/spgxlog.h +++ b/src/include/access/spgxlog.h @@ -35,7 +35,7 @@ */ typedef struct spgxlogState { - TransactionId myXid; + TransactionId redirectXid; bool isBuild; } spgxlogState; |