From 0474461b71420d7331d0d59b84ae497ffb20f0d1 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 4 Aug 2020 22:04:57 -0400 Subject: [PATCH v4 2/2] Avoid spurious CREATE INDEX CONCURRENTLY waits --- src/backend/commands/indexcmds.c | 92 ++++++++++++++++++++++++++++++-- src/include/storage/proc.h | 6 ++- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 75552c64ed..4abb60ea44 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -94,6 +94,7 @@ static void ReindexMultipleInternal(List *relids, int options); static void reindex_error_callback(void *args); static void update_relispartition(Oid relationId, bool newval); static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts); +static void set_safe_index_flag(void); /* * callback argument type for RangeVarCallbackForReindexIndex() @@ -385,7 +386,10 @@ 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.) + * 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. * * Also, GetCurrentVirtualXIDs never reports our own vxid, so we need not * check for that. @@ -406,7 +410,8 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress) VirtualTransactionId *old_snapshots; old_snapshots = GetCurrentVirtualXIDs(limitXmin, true, false, - PROC_IS_AUTOVACUUM | PROC_IN_VACUUM, + PROC_IS_AUTOVACUUM | PROC_IN_VACUUM + | PROC_IN_SAFE_IC, &n_old_snapshots); if (progress) pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, n_old_snapshots); @@ -426,7 +431,8 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress) newer_snapshots = GetCurrentVirtualXIDs(limitXmin, true, false, - PROC_IS_AUTOVACUUM | PROC_IN_VACUUM, + PROC_IS_AUTOVACUUM | PROC_IN_VACUUM + | PROC_IN_SAFE_IC, &n_newer_snapshots); for (j = i; j < n_old_snapshots; j++) { @@ -519,6 +525,7 @@ DefineIndex(Oid relationId, bool amcanorder; amoptions_function amoptions; bool partitioned; + bool safe_index; Datum reloptions; int16 *coloptions; IndexInfo *indexInfo; @@ -1045,6 +1052,17 @@ DefineIndex(Oid relationId, } } + /* + * When doing concurrent index builds, we can set a PGPROC flag to tell + * concurrent VACUUM, CREATE INDEX CONCURRENTLY and REINDEX CONCURRENTLY + * to ignore us when waiting for concurrent snapshots. That can only be + * done for indexes that don't execute any expressions. Determine that. + * (The flag is reset automatically at transaction end, so it must be + * set for each transaction.) + */ + safe_index = indexInfo->ii_Expressions == NIL && + indexInfo->ii_Predicate == NIL; + /* * Report index creation if appropriate (delay this till after most of the * error checks) @@ -1431,6 +1449,10 @@ DefineIndex(Oid relationId, CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* * The index is now visible, so we can report the OID. */ @@ -1490,6 +1512,10 @@ DefineIndex(Oid relationId, CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* * Phase 3 of concurrent index build * @@ -1546,6 +1572,10 @@ DefineIndex(Oid relationId, CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* We should now definitely not be advertising any xmin. */ Assert(MyProc->xmin == InvalidTransactionId); @@ -3021,6 +3051,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) PROGRESS_CREATEIDX_ACCESS_METHOD_OID }; int64 progress_vals[4]; + bool safe_index = true; /* * Create a memory context that will survive forced transaction commits we @@ -3324,6 +3355,23 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ newIndexRel = index_open(newIndexId, ShareUpdateExclusiveLock); + /* + * When doing concurrent reindex, we can set a PGPROC flag to tell + * concurrent VACUUM, CREATE INDEX CONCURRENTLY and REINDEX + * CONCURRENTLY to ignore us when waiting for concurrent snapshots. + * That can only be done for indexes that don't execute any + * expressions. Determine that for all involved indexes together. (The + * flag is reset automatically at transaction end, so it must be set + * for each transaction.) + */ + if (safe_index) + { + IndexInfo *newIndexInfo = BuildIndexInfo(newIndexRel); + + safe_index = newIndexInfo->ii_Expressions == NIL && + newIndexInfo->ii_Predicate == NIL; + } + /* * Save the list of OIDs and locks in private context */ @@ -3393,6 +3441,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* * Phase 2 of REINDEX CONCURRENTLY * @@ -3456,6 +3508,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) } StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* * Phase 3 of REINDEX CONCURRENTLY * @@ -3559,6 +3615,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + forboth(lc, indexIds, lc2, newIndexIds) { char *oldName; @@ -3609,6 +3669,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* * Phase 5 of REINDEX CONCURRENTLY * @@ -3641,6 +3705,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* * Phase 6 of REINDEX CONCURRENTLY * @@ -3692,6 +3760,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) /* Start a new transaction to finish process properly */ StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (safe_index) + set_safe_index_flag(); + /* Log what we did */ if (options & REINDEXOPT_VERBOSE) { @@ -3896,3 +3968,17 @@ update_relispartition(Oid relationId, bool newval) heap_freetuple(tup); table_close(classRel, RowExclusiveLock); } + +/* + * Set a PGPROC flag to tell concurrent VACUUM, CREATE INDEX CONCURRENTLY and + * REINDEX CONCURRENTLY to ignore us when waiting for concurrent snapshots. + * Should be called just after starting a transaction. + */ +static void +set_safe_index_flag() +{ + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE); + MyProc->statusFlags |= PROC_IN_SAFE_IC; + ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags; + LWLockRelease(ProcArrayLock); +} diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 63aea0e253..6664803e2a 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -53,13 +53,17 @@ 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 */ #define PROC_IN_LOGICAL_DECODING 0x10 /* currently doing logical * decoding outside xact */ /* flags reset at EOXact */ #define PROC_VACUUM_STATE_MASK \ - (PROC_IN_VACUUM | PROC_VACUUM_FOR_WRAPAROUND) + (PROC_IN_VACUUM | PROC_IN_SAFE_IC | PROC_VACUUM_FOR_WRAPAROUND) /* * We allow a small number of "weak" relation locks (AccessShareLock, -- 2.21.0