diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index ca24620fd0..a52cb16b5e 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -114,6 +114,17 @@ typedef struct ReindexErrorInfo char relkind; } ReindexErrorInfo; +/* + * Index to process in ReindexRelationConcurrently + */ +typedef struct ReindexIndexInfo +{ + Oid indexId; + Oid tableId; + Oid amId; + bool safe; /* for set_indexsafe_procflags */ +} ReindexIndexInfo; + /* * CheckIndexCompatible * Determine whether an existing index definition is compatible with a @@ -385,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. @@ -1564,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); @@ -3132,10 +3145,16 @@ ReindexRelationConcurrently(Oid relationOid, int options) get_rel_name(cellOid)))); else { + ReindexIndexInfo *idx; + /* Save the list of relation OIDs in private context */ oldcontext = MemoryContextSwitchTo(private_context); - indexIds = lappend_oid(indexIds, cellOid); + idx = palloc(sizeof(ReindexIndexInfo)); + idx->indexId = cellOid; + /* other fields set later */ + + indexIds = lappend(indexIds, idx); MemoryContextSwitchTo(oldcontext); } @@ -3172,13 +3191,18 @@ ReindexRelationConcurrently(Oid relationOid, int options) get_rel_name(cellOid)))); else { + ReindexIndexInfo *idx; + /* * Save the list of relation OIDs in private * context */ oldcontext = MemoryContextSwitchTo(private_context); - indexIds = lappend_oid(indexIds, cellOid); + idx = palloc(sizeof(ReindexIndexInfo)); + idx->indexId = cellOid; + indexIds = lappend(indexIds, idx); + /* other fields set later */ MemoryContextSwitchTo(oldcontext); } @@ -3197,6 +3221,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) Oid heapId = IndexGetRelation(relationOid, (options & REINDEXOPT_MISSING_OK) != 0); Relation heapRelation; + ReindexIndexInfo *idx; /* if relation is missing, leave */ if (!OidIsValid(heapId)) @@ -3247,7 +3272,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) * Save the list of relation OIDs in private context. Note * that invalid indexes are allowed here. */ - indexIds = lappend_oid(indexIds, relationOid); + idx = palloc(sizeof(ReindexIndexInfo)); + idx->indexId = relationOid; + indexIds = lappend(indexIds, idx); + /* other fields set later */ MemoryContextSwitchTo(oldcontext); break; @@ -3306,31 +3334,40 @@ ReindexRelationConcurrently(Oid relationOid, int options) foreach(lc, indexIds) { char *concurrentName; - Oid indexId = lfirst_oid(lc); + ReindexIndexInfo *idx = lfirst(lc); + ReindexIndexInfo *newidx; Oid newIndexId; Relation indexRel; Relation heapRel; Relation newIndexRel; LockRelId *lockrelid; - indexRel = index_open(indexId, ShareUpdateExclusiveLock); + /* XXX pointless lock acquisition */ + indexRel = index_open(idx->indexId, ShareUpdateExclusiveLock); 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; + /* This function shouldn't be called for temporary relations. */ if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP) elog(ERROR, "cannot reindex a temporary table concurrently"); pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, - RelationGetRelid(heapRel)); + idx->tableId); + progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY; progress_vals[1] = 0; /* initializing */ - progress_vals[2] = indexId; - progress_vals[3] = indexRel->rd_rel->relam; + progress_vals[2] = idx->indexId; + progress_vals[3] = idx->amId; pgstat_progress_update_multi_param(4, progress_index, progress_vals); /* Choose a temporary relation name for the new index */ - concurrentName = ChooseRelationName(get_rel_name(indexId), + concurrentName = ChooseRelationName(get_rel_name(idx->indexId), NULL, "ccnew", get_rel_namespace(indexRel->rd_index->indrelid), @@ -3338,7 +3375,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) /* Create new index definition based on given index */ newIndexId = index_concurrently_create_copy(heapRel, - indexId, + idx->indexId, concurrentName); /* @@ -3352,7 +3389,13 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ oldcontext = MemoryContextSwitchTo(private_context); - newIndexIds = lappend_oid(newIndexIds, newIndexId); + newidx = palloc(sizeof(ReindexIndexInfo)); + newidx->indexId = newIndexId; + newidx->safe = idx->safe; + newidx->tableId = idx->tableId; + newidx->amId = idx->amId; + + newIndexIds = lappend(newIndexIds, newidx); /* * Save lockrelid to protect each relation from drop then close @@ -3380,6 +3423,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ foreach(lc, heapRelationIds) { + /* XXX pointless lock acquisition */ Relation heapRelation = table_open(lfirst_oid(lc), ShareUpdateExclusiveLock); LockRelId *lockrelid; LOCKTAG *heaplocktag; @@ -3416,6 +3460,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 * @@ -3433,10 +3482,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) foreach(lc, newIndexIds) { - Relation newIndexRel; - Oid newIndexId = lfirst_oid(lc); - Oid heapId; - Oid indexam; + ReindexIndexInfo *newidx = lfirst(lc); /* Start new transaction for this index's concurrent build */ StartTransactionCommand(); @@ -3448,37 +3494,38 @@ 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()); - /* - * Index relation has been closed by previous commit, so reopen it to - * get its information. - */ - newIndexRel = index_open(newIndexId, ShareUpdateExclusiveLock); - heapId = newIndexRel->rd_index->indrelid; - indexam = newIndexRel->rd_rel->relam; - index_close(newIndexRel, NoLock); - /* * Update progress for the index to build, with the correct parent * table involved. */ - pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, heapId); + pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, newidx->tableId); progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY; progress_vals[1] = PROGRESS_CREATEIDX_PHASE_BUILD; - progress_vals[2] = newIndexId; - progress_vals[3] = indexam; + progress_vals[2] = newidx->indexId; + progress_vals[3] = newidx->amId; pgstat_progress_update_multi_param(4, progress_index, progress_vals); /* Perform concurrent build of new index */ - index_concurrently_build(heapId, newIndexId); + index_concurrently_build(newidx->tableId, newidx->indexId); 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 * @@ -3494,12 +3541,9 @@ ReindexRelationConcurrently(Oid relationOid, int options) foreach(lc, newIndexIds) { - Oid newIndexId = lfirst_oid(lc); - Oid heapId; + ReindexIndexInfo *newidx = lfirst(lc); TransactionId limitXmin; Snapshot snapshot; - Relation newIndexRel; - Oid indexam; StartTransactionCommand(); @@ -3510,6 +3554,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. @@ -3517,27 +3565,19 @@ ReindexRelationConcurrently(Oid relationOid, int options) snapshot = RegisterSnapshot(GetTransactionSnapshot()); PushActiveSnapshot(snapshot); - /* - * Index relation has been closed by previous commit, so reopen it to - * get its information. - */ - newIndexRel = index_open(newIndexId, ShareUpdateExclusiveLock); - heapId = newIndexRel->rd_index->indrelid; - indexam = newIndexRel->rd_rel->relam; - index_close(newIndexRel, NoLock); - /* * Update progress for the index to build, with the correct parent * table involved. */ - pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, heapId); + pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, + newidx->tableId); progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY; progress_vals[1] = PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN; - progress_vals[2] = newIndexId; - progress_vals[3] = indexam; + progress_vals[2] = newidx->indexId; + progress_vals[3] = newidx->amId; pgstat_progress_update_multi_param(4, progress_index, progress_vals); - validate_index(heapId, newIndexId, snapshot); + validate_index(newidx->tableId, newidx->indexId, snapshot); /* * We can now do away with our active snapshot, we still need to save @@ -3552,6 +3592,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(); @@ -3582,12 +3625,18 @@ 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); + ReindexIndexInfo *newidx = lfirst(lc2); char *oldName; - Oid oldIndexId = lfirst_oid(lc); - Oid newIndexId = lfirst_oid(lc2); - Oid heapId; /* * Check for user-requested abort. This is inside a transaction so as @@ -3596,27 +3645,25 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ CHECK_FOR_INTERRUPTS(); - heapId = IndexGetRelation(oldIndexId, false); - /* Choose a relation name for old index */ - oldName = ChooseRelationName(get_rel_name(oldIndexId), + oldName = ChooseRelationName(get_rel_name(oldidx->indexId), NULL, "ccold", - get_rel_namespace(heapId), + get_rel_namespace(oldidx->tableId), false); /* * Swap old index with the new one. This also marks the new one as * valid and the old one as not valid. */ - index_concurrently_swap(newIndexId, oldIndexId, oldName); + index_concurrently_swap(newidx->indexId, oldidx->indexId, oldName); /* * Invalidate the relcache for the table, so that after this commit * all sessions will refresh any cached plans that might reference the * index. */ - CacheInvalidateRelcacheByRelid(heapId); + CacheInvalidateRelcacheByRelid(oldidx->tableId); /* * CCI here so that subsequent iterations see the oldName in the @@ -3632,6 +3679,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 * @@ -3646,8 +3699,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) foreach(lc, indexIds) { - Oid oldIndexId = lfirst_oid(lc); - Oid heapId; + ReindexIndexInfo *oldidx = lfirst(lc); /* * Check for user-requested abort. This is inside a transaction so as @@ -3656,14 +3708,19 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ CHECK_FOR_INTERRUPTS(); - heapId = IndexGetRelation(oldIndexId, false); - index_concurrently_set_dead(heapId, oldIndexId); + index_concurrently_set_dead(oldidx->tableId, oldidx->indexId); } /* Commit this transaction to make the updates visible. */ 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 * @@ -3681,11 +3738,11 @@ ReindexRelationConcurrently(Oid relationOid, int options) foreach(lc, indexIds) { - Oid oldIndexId = lfirst_oid(lc); + ReindexIndexInfo *idx = lfirst(lc); ObjectAddress object; object.classId = RelationRelationId; - object.objectId = oldIndexId; + object.objectId = idx->indexId; object.objectSubId = 0; add_exact_object_address(&object, objects); @@ -3728,7 +3785,8 @@ ReindexRelationConcurrently(Oid relationOid, int options) { foreach(lc, newIndexIds) { - Oid indOid = lfirst_oid(lc); + ReindexIndexInfo *idx = lfirst(lc); + Oid indOid = idx->indexId; ereport(INFO, (errmsg("index \"%s.%s\" was reindexed", 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 */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index b8ca8cffd9..f2df682a9d 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2046,6 +2046,7 @@ Regis RegisNode RegisteredBgWorker ReindexErrorInfo +ReindexIndexInfo ReindexObjectType ReindexStmt ReindexType