From 54627ab4e4050e796216574c3afda58e496e305e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 30 Nov 2020 16:01:46 -0300 Subject: [PATCH 2/2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY --- src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++--- src/include/storage/proc.h | 1 + 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index b1ce83e1dd..c5f257ce15 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -122,6 +122,7 @@ typedef struct ReindexIndexInfo Oid indexId; Oid tableId; Oid amId; + bool safe; /* for set_indexsafe_procflags */ } ReindexIndexInfo; /* @@ -395,7 +396,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts) * lazy VACUUMs, because they won't be fazed by missing index entries * either. (Manual ANALYZEs, however, can't be excluded because they * might be within transactions that are going to do arbitrary operations - * later.) Processes running CREATE INDEX CONCURRENTLY + * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY * on indexes that are neither expressional nor partial are also safe to * ignore, since we know that those processes won't examine any data * outside the table they're indexing. @@ -1574,9 +1575,11 @@ DefineIndex(Oid relationId, CommitTransactionCommand(); StartTransactionCommand(); - /* Tell concurrent index builds to ignore us, if index qualifies */ - if (safe_index) - set_indexsafe_procflags(); + /* + * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because + * it only takes a snapshot to do some catalog manipulations, after the + * wait is over. + */ /* We should now definitely not be advertising any xmin. */ Assert(MyProc->xmin == InvalidTransactionId); @@ -3343,6 +3346,9 @@ ReindexRelationConcurrently(Oid relationOid, int options) heapRel = table_open(indexRel->rd_index->indrelid, ShareUpdateExclusiveLock); + /* determine safety of this index for set_indexsafe_procflags */ + idx->safe = (indexRel->rd_indexprs == NIL && + indexRel->rd_indpred == NIL); idx->tableId = RelationGetRelid(heapRel); idx->amId = indexRel->rd_rel->relam; @@ -3384,6 +3390,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) newidx = palloc(sizeof(ReindexIndexInfo)); newidx->indexId = newIndexId; + newidx->safe = idx->safe; newidx->tableId = idx->tableId; newidx->amId = idx->amId; @@ -3451,6 +3458,11 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* + * Because we don't take a snapshot in this transaction, there's no need + * to set the PROC_IN_SAFE_IC flag here. + */ + /* * Phase 2 of REINDEX CONCURRENTLY * @@ -3480,6 +3492,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ CHECK_FOR_INTERRUPTS(); + /* Tell concurrent indexing to ignore us, if index qualifies */ + if (newidx->safe) + set_indexsafe_procflags(); + /* Set ActiveSnapshot since functions in the indexes may need it */ PushActiveSnapshot(GetTransactionSnapshot()); @@ -3500,8 +3516,14 @@ ReindexRelationConcurrently(Oid relationOid, int options) PopActiveSnapshot(); CommitTransactionCommand(); } + StartTransactionCommand(); + /* + * Because we don't take a snapshot in this transaction, there's no need + * to set the PROC_IN_SAFE_IC flag here. + */ + /* * Phase 3 of REINDEX CONCURRENTLY * @@ -3530,6 +3552,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ CHECK_FOR_INTERRUPTS(); + /* Tell concurrent indexing to ignore us, if index qualifies */ + if (newidx->safe) + set_indexsafe_procflags(); + /* * Take the "reference snapshot" that will be used by validate_index() * to filter candidate tuples. @@ -3564,6 +3590,9 @@ ReindexRelationConcurrently(Oid relationOid, int options) * To ensure no deadlocks, we must commit and start yet another * transaction, and do our wait before any snapshot has been taken in * it. + * + * Because we don't take a snapshot in this transaction, there's no + * need to set the PROC_IN_SAFE_IC flag here. */ CommitTransactionCommand(); StartTransactionCommand(); @@ -3594,6 +3623,13 @@ ReindexRelationConcurrently(Oid relationOid, int options) StartTransactionCommand(); + /* + * Because this transaction only does catalog manipulations and doesn't do + * any index operations, we can set the PROC_IN_SAFE_IC flag here + * unconditionally. + */ + set_indexsafe_procflags(); + forboth(lc, indexIds, lc2, newIndexIds) { ReindexIndexInfo *oldidx = lfirst(lc); @@ -3641,6 +3677,12 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* + * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because + * it only takes a snapshot to do some catalog manipulations, after the + * wait is over. + */ + /* * Phase 5 of REINDEX CONCURRENTLY * @@ -3671,6 +3713,12 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* + * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because + * it only takes a snapshot to do some catalog manipulations, after all + * the waiting has been completed. + */ + /* * Phase 6 of REINDEX CONCURRENTLY * diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index e77f76ae8a..e1a6bc5170 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -54,6 +54,7 @@ struct XidCache #define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */ #define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */ #define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX + * CONCURRENTLY or REINDEX * CONCURRENTLY on non-expressional, * non-partial index */ #define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */ -- 2.20.1