From 1e5560d4a3f3e3b4cb83803f90985701fb4dee8c Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 4 Aug 2020 22:04:57 -0400 Subject: [PATCH v5] Avoid spurious CREATE INDEX CONCURRENTLY waits --- src/backend/commands/indexcmds.c | 85 +++++++++++++++++++++++++++++--- src/include/storage/proc.h | 6 ++- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 35696f9f75..44ea84c54d 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -68,6 +68,7 @@ /* non-export function prototypes */ +static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts); static void CheckPredicate(Expr *predicate); static void ComputeIndexAttrs(IndexInfo *indexInfo, Oid *typeOidP, @@ -87,13 +88,12 @@ static char *ChooseIndexNameAddition(List *colnames); static List *ChooseIndexColumnNames(List *indexElems); static void RangeVarCallbackForReindexIndex(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); -static bool ReindexRelationConcurrently(Oid relationOid, int options); - +static void reindex_error_callback(void *args); static void ReindexPartitions(Oid relid, int options, bool isTopLevel); static void ReindexMultipleInternal(List *relids, int options); -static void reindex_error_callback(void *args); +static bool ReindexRelationConcurrently(Oid relationOid, int options); static void update_relispartition(Oid relationId, bool newval); -static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts); +static inline void set_safe_index_flag(void); /* * callback argument type for RangeVarCallbackForReindexIndex() @@ -385,7 +385,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 +409,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 +430,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 +524,7 @@ DefineIndex(Oid relationId, bool amcanorder; amoptions_function amoptions; bool partitioned; + bool safe_index; Datum reloptions; int16 *coloptions; IndexInfo *indexInfo; @@ -1045,6 +1051,10 @@ DefineIndex(Oid relationId, } } + /* Determine whether we can call set_safe_index_flag */ + 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 +1441,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 +1504,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 +1564,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 +3043,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) PROGRESS_CREATEIDX_ACCESS_METHOD_OID }; int64 progress_vals[4]; + bool all_indexes_safe = true; /* * Create a memory context that will survive forced transaction commits we @@ -3325,6 +3348,12 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ newIndexRel = index_open(newIndexId, ShareUpdateExclusiveLock); + /* consider safety of this index for set_safe_index_flag */ + if (all_indexes_safe && + (newIndexRel->rd_indexprs != NIL || + newIndexRel->rd_indpred != NIL)) + all_indexes_safe = false; + /* * Save the list of OIDs and locks in private context */ @@ -3394,6 +3423,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (all_indexes_safe) + set_safe_index_flag(); + /* * Phase 2 of REINDEX CONCURRENTLY * @@ -3457,6 +3490,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) } StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (all_indexes_safe) + set_safe_index_flag(); + /* * Phase 3 of REINDEX CONCURRENTLY * @@ -3610,6 +3647,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (all_indexes_safe) + set_safe_index_flag(); + /* * Phase 5 of REINDEX CONCURRENTLY * @@ -3642,6 +3683,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) CommitTransactionCommand(); StartTransactionCommand(); + /* Tell concurrent index builds to ignore us, if index qualifies */ + if (all_indexes_safe) + set_safe_index_flag(); + /* * Phase 6 of REINDEX CONCURRENTLY * @@ -3897,3 +3942,29 @@ update_relispartition(Oid relationId, bool newval) heap_freetuple(tup); table_close(classRel, RowExclusiveLock); } + +/* + * Set the PROC_IN_SAFE_IC flag in my PGPROC entry. + * + * When doing concurrent index builds, we can set this flag + * to tell other processes concurrently running VACUUM, CREATE + * INDEX CONCURRENTLY and REINDEX CONCURRENTLY to ignore us when + * doing their waits for concurrent snapshots. On one hand it + * avoids pointlessly waiting for a process that's not interesting + * anyway, but more importantly it avoids deadlocks in some cases. + * + * This can only be done for indexes that don't execute any expressions. + * Caller is responsible for only calling this routine when that + * assumption holds true. + * + * (The flag is reset automatically at transaction end, so it must be + * set for each transaction.) + */ +static inline void +set_safe_index_flag(void) +{ + 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 1067f58f51..b2347ffd79 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.20.1