From 2cb0d06c709c06f7876cde6a340fc1baf4664fc4 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 1 Nov 2020 21:47:11 -0600 Subject: [PATCH 2/9] Refactor to allow reindexing all index partitions at once --- src/backend/commands/indexcmds.c | 369 +++++++++++++++++++------------ 1 file changed, 222 insertions(+), 147 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index a3ba24947e..423c5fd78a 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -88,6 +88,8 @@ 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 List *ReindexIndexesConcurrently(List *indexIds, int options, + MemoryContext private_context); static void ReindexPartitions(Oid relid, int options, bool isTopLevel); static void ReindexMultipleInternal(List *relids, int options); @@ -1404,7 +1406,8 @@ DefineIndex(Oid relationId, pgstat_progress_update_param(PROGRESS_CREATEIDX_INDEX_OID, indexRelationId); - ReindexRelationConcurrently(indexRelationId, REINDEXOPT_CONCURRENTLY); + ReindexIndexesConcurrently(list_make1_oid(indexRelationId), + REINDEXOPT_CONCURRENTLY, CurrentMemoryContext); pgstat_progress_end_command(); @@ -2296,7 +2299,7 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel) ReindexPartitions(indOid, options, isTopLevel); else if ((options & REINDEXOPT_CONCURRENTLY) != 0 && persistence != RELPERSISTENCE_TEMP) - ReindexRelationConcurrently(indOid, options); + ReindexIndexesConcurrently(list_make1_oid(indOid), options, CurrentMemoryContext); else reindex_index(indOid, false, persistence, options | REINDEXOPT_REPORT_PROGRESS); @@ -2620,6 +2623,47 @@ reindex_error_callback(void *arg) errinfo->relnamespace, errinfo->relname); } + +/* + * Given a list of index oids, return a list of leaf partitions by removing + * any intermediate parents. heaprels is populated with the corresponding + * tables. + */ +static List * +leaf_partitions(List *inhoids, int options, List **heaprels) +{ + List *partitions = NIL; + ListCell *lc; + + foreach(lc, inhoids) + { + Oid partoid = lfirst_oid(lc); + Oid tableoid; + Relation table; + char partkind = get_rel_relkind(partoid); + + /* + * This discards partitioned indexes and foreign tables. + */ + if (!RELKIND_HAS_STORAGE(partkind)) + continue; + + Assert(partkind == RELKIND_INDEX); + + /* (try to) Open the table, with lock */ + tableoid = IndexGetRelation(partoid, false); + table = table_open(tableoid, ShareLock); + table_close(table, NoLock); + + /* Save partition OID in current MemoryContext */ + partitions = lappend_oid(partitions, partoid); + *heaprels = lappend_oid(*heaprels, tableoid); + } + + return partitions; +} + + /* * ReindexPartitions * @@ -2629,11 +2673,13 @@ reindex_error_callback(void *arg) static void ReindexPartitions(Oid relid, int options, bool isTopLevel) { - List *partitions = NIL; + List *partitions = NIL, + *heaprels = NIL; char relkind = get_rel_relkind(relid); char *relname = get_rel_name(relid); char *relnamespace = get_namespace_name(get_rel_namespace(relid)); MemoryContext reindex_context; + MemoryContext old_context; List *inhoids; ListCell *lc; ErrorContextCallback errcallback; @@ -2678,33 +2724,42 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel) * The list of relations to reindex are the physical partitions of the * tree so discard any partitioned table or index. */ - foreach(lc, inhoids) - { - Oid partoid = lfirst_oid(lc); - char partkind = get_rel_relkind(partoid); - MemoryContext old_context; - /* - * This discards partitioned tables, partitioned indexes and foreign - * tables. - */ - if (!RELKIND_HAS_STORAGE(partkind)) - continue; - - Assert(partkind == RELKIND_INDEX || - partkind == RELKIND_RELATION); - - /* Save partition OID */ + if (relkind == RELKIND_PARTITIONED_INDEX) + { old_context = MemoryContextSwitchTo(reindex_context); - partitions = lappend_oid(partitions, partoid); + partitions = leaf_partitions(inhoids, options, &heaprels); MemoryContextSwitchTo(old_context); + } else { + /* Loop over parent tables */ + foreach(lc, inhoids) + { + Oid partoid = lfirst_oid(lc); + Relation parttable; + List *partindexes; + + parttable = table_open(partoid, ShareLock); + old_context = MemoryContextSwitchTo(reindex_context); + partindexes = RelationGetIndexList(parttable); + partindexes = leaf_partitions(partindexes, options, &heaprels); + partitions = list_concat(partitions, partindexes); + MemoryContextSwitchTo(old_context); + table_close(parttable, ShareLock); + } } - /* - * Process each partition listed in a separate transaction. Note that - * this commits and then starts a new transaction immediately. - */ - ReindexMultipleInternal(partitions, options); + if ((options & REINDEXOPT_CONCURRENTLY) != 0 && + get_rel_persistence(relid) != RELPERSISTENCE_TEMP) + { + /* Process all indexes in a single loop */ + ReindexIndexesConcurrently(partitions, options, reindex_context); + } else { + /* + * Process each partition listed in a separate transaction. Note that + * this commits and then starts a new transaction immediately. + */ + ReindexMultipleInternal(partitions, options); + } /* * Clean up working storage --- note we must do this after @@ -2806,11 +2861,10 @@ ReindexMultipleInternal(List *relids, int options) * ReindexRelationConcurrently - process REINDEX CONCURRENTLY for given * relation OID * - * 'relationOid' can either belong to an index, a table or a materialized + * 'relationOid' can either belong to a table or a materialized * view. For tables and materialized views, all its indexes will be rebuilt, * excluding invalid indexes and any indexes used in exclusion constraints, - * but including its associated toast table indexes. For indexes, the index - * itself will be rebuilt. + * but including its associated toast table indexes. * * The locks taken on parent tables and involved indexes are kept until the * transaction is committed, at which point a session lock is taken on each @@ -2828,11 +2882,8 @@ ReindexMultipleInternal(List *relids, int options) static bool ReindexRelationConcurrently(Oid relationOid, int options) { - List *heapRelationIds = NIL; List *indexIds = NIL; List *newIndexIds = NIL; - List *relationLocks = NIL; - List *lockTags = NIL; ListCell *lc, *lc2; MemoryContext private_context; @@ -2841,13 +2892,6 @@ ReindexRelationConcurrently(Oid relationOid, int options) char *relationName = NULL; char *relationNamespace = NULL; PGRUsage ru0; - const int progress_index[] = { - PROGRESS_CREATEIDX_COMMAND, - PROGRESS_CREATEIDX_PHASE, - PROGRESS_CREATEIDX_INDEX_OID, - PROGRESS_CREATEIDX_ACCESS_METHOD_OID - }; - int64 progress_vals[4]; /* * Create a memory context that will survive forced transaction commits we @@ -2890,14 +2934,6 @@ ReindexRelationConcurrently(Oid relationOid, int options) */ Relation heapRelation; - /* Save the list of relation OIDs in private context */ - oldcontext = MemoryContextSwitchTo(private_context); - - /* Track this relation for session locks */ - heapRelationIds = lappend_oid(heapRelationIds, relationOid); - - MemoryContextSwitchTo(oldcontext); - if (IsCatalogRelationOid(relationOid)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -2955,14 +2991,6 @@ ReindexRelationConcurrently(Oid relationOid, int options) Relation toastRelation = table_open(toastOid, ShareUpdateExclusiveLock); - /* Save the list of relation OIDs in private context */ - oldcontext = MemoryContextSwitchTo(private_context); - - /* Track this relation for session locks */ - heapRelationIds = lappend_oid(heapRelationIds, toastOid); - - MemoryContextSwitchTo(oldcontext); - foreach(lc2, RelationGetIndexList(toastRelation)) { Oid cellOid = lfirst_oid(lc2); @@ -2997,66 +3025,8 @@ ReindexRelationConcurrently(Oid relationOid, int options) table_close(heapRelation, NoLock); break; } - case RELKIND_INDEX: - { - Oid heapId = IndexGetRelation(relationOid, - (options & REINDEXOPT_MISSING_OK) != 0); - Relation heapRelation; - - /* if relation is missing, leave */ - if (!OidIsValid(heapId)) - break; - - if (IsCatalogRelationOid(heapId)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot reindex system catalogs concurrently"))); - - /* - * Don't allow reindex for an invalid index on TOAST table, as - * if rebuilt it would not be possible to drop it. - */ - if (IsToastNamespace(get_rel_namespace(relationOid)) && - !get_index_isvalid(relationOid)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot reindex invalid index on TOAST table concurrently"))); - - /* - * Check if parent relation can be locked and if it exists, - * this needs to be done at this stage as the list of indexes - * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK - * should not be used once all the session locks are taken. - */ - if ((options & REINDEXOPT_MISSING_OK) != 0) - { - heapRelation = try_table_open(heapId, - ShareUpdateExclusiveLock); - /* leave if relation does not exist */ - if (!heapRelation) - break; - } - else - heapRelation = table_open(heapId, - ShareUpdateExclusiveLock); - table_close(heapRelation, NoLock); - - /* Save the list of relation OIDs in private context */ - oldcontext = MemoryContextSwitchTo(private_context); - - /* Track the heap relation of this index for session locks */ - heapRelationIds = list_make1_oid(heapId); - - /* - * Save the list of relation OIDs in private context. Note - * that invalid indexes are allowed here. - */ - indexIds = lappend_oid(indexIds, relationOid); - - MemoryContextSwitchTo(oldcontext); - break; - } + case RELKIND_INDEX: case RELKIND_PARTITIONED_TABLE: case RELKIND_PARTITIONED_INDEX: default: @@ -3080,7 +3050,144 @@ ReindexRelationConcurrently(Oid relationOid, int options) return false; } - Assert(heapRelationIds != NIL); + /* Do the work */ + newIndexIds = ReindexIndexesConcurrently(indexIds, options, private_context); + if (newIndexIds == NIL) + return false; + + /* Log what we did */ + if (options & REINDEXOPT_VERBOSE) + { + if (relkind == RELKIND_INDEX) + ereport(INFO, + (errmsg("index \"%s.%s\" was reindexed", + relationNamespace, relationName), + errdetail("%s.", + pg_rusage_show(&ru0)))); + else + { + foreach(lc, newIndexIds) + { + Oid indOid = lfirst_oid(lc); + + ereport(INFO, + (errmsg("index \"%s.%s\" was reindexed", + get_namespace_name(get_rel_namespace(indOid)), + get_rel_name(indOid)))); + /* Don't show rusage here, since it's not per index. */ + } + + ereport(INFO, + (errmsg("table \"%s.%s\" was reindexed", + relationNamespace, relationName), + errdetail("%s.", + pg_rusage_show(&ru0)))); + } + } + + MemoryContextDelete(private_context); + + return true; +} + +/* + * Reindex concurrently for an list of index relations + * This is called by DefineIndex, ReindexPartitions, and + * ReindexRelationConcurrently. + * indexIds should be a list of "related" indexes, like all indexes on a + * relation, or all partitions of a partitioned index. Expect deadlocks if + * passed unrelated indexes. + */ +static List * +ReindexIndexesConcurrently(List *indexIds, int options, + MemoryContext private_context) +{ + List *heapRelationIds = NIL; + List *newIndexIds = NIL; + List *relationLocks = NIL; + List *lockTags = NIL; + + ListCell *lc, + *lc2; + + MemoryContext oldcontext; + + const int progress_index[] = { + PROGRESS_CREATEIDX_COMMAND, + PROGRESS_CREATEIDX_PHASE, + PROGRESS_CREATEIDX_INDEX_OID, + PROGRESS_CREATEIDX_ACCESS_METHOD_OID + }; + int64 progress_vals[4]; + + foreach(lc, indexIds) + { + Oid indexrelid = lfirst_oid(lc); + Oid heapId = IndexGetRelation(indexrelid, + (options & REINDEXOPT_MISSING_OK) != 0); + Relation heapRelation; + + /* if relation is missing, leave */ + if (!OidIsValid(heapId)) + { + indexIds = list_delete_cell(indexIds, lc); + continue; + // This is the case of either: 1) a single index being reindexed, + // where its relation has disappeared; or, 2) a concurrent reindex + // of an partitioned index, for which one of the tables has been + // dropped. XXX: can that happen with locks held ?? + } + + if (IsCatalogRelationOid(heapId)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot reindex system catalogs concurrently"))); + + /* + * Don't allow reindex for an invalid index on TOAST table, as + * if rebuilt it would not be possible to drop it. + */ + if (IsToastNamespace(get_rel_namespace(indexrelid)) && + !get_index_isvalid(indexrelid)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot reindex invalid index on TOAST table concurrently"))); + + /* + * Check if parent relation can be locked and if it exists, + * this needs to be done at this stage as the list of indexes + * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK + * should not be used once all the session locks are taken. + */ + if ((options & REINDEXOPT_MISSING_OK) != 0) + { + heapRelation = try_table_open(heapId, + ShareUpdateExclusiveLock); + /* leave if relation does not exist */ + if (!heapRelation) + { + indexIds = list_delete_cell(indexIds, lc); + continue; + } + } + else + heapRelation = table_open(heapId, + ShareUpdateExclusiveLock); + table_close(heapRelation, NoLock); + + /* Save the list of relation OIDs in private context */ + oldcontext = MemoryContextSwitchTo(private_context); + + /* Track the heap relation of this index for session locks */ + heapRelationIds = lappend_oid(heapRelationIds, heapId); + + /* Note that invalid indexes are allowed here. */ + + MemoryContextSwitchTo(oldcontext); + } + + if (indexIds == NIL) + return NIL; /*----- * Now we have all the indexes we want to process in indexIds. @@ -3519,41 +3626,9 @@ ReindexRelationConcurrently(Oid relationOid, int options) /* Start a new transaction to finish process properly */ StartTransactionCommand(); - /* Log what we did */ - if (options & REINDEXOPT_VERBOSE) - { - if (relkind == RELKIND_INDEX) - ereport(INFO, - (errmsg("index \"%s.%s\" was reindexed", - relationNamespace, relationName), - errdetail("%s.", - pg_rusage_show(&ru0)))); - else - { - foreach(lc, newIndexIds) - { - Oid indOid = lfirst_oid(lc); - - ereport(INFO, - (errmsg("index \"%s.%s\" was reindexed", - get_namespace_name(get_rel_namespace(indOid)), - get_rel_name(indOid)))); - /* Don't show rusage here, since it's not per index. */ - } - - ereport(INFO, - (errmsg("table \"%s.%s\" was reindexed", - relationNamespace, relationName), - errdetail("%s.", - pg_rusage_show(&ru0)))); - } - } - - MemoryContextDelete(private_context); - pgstat_progress_end_command(); - return true; + return newIndexIds; } /* -- 2.17.0