agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
Subject: [PATCH v13 05/18] Refactor to allow reindexing all index partitions at once..
Date: Sun, 1 Nov 2020 12:25:15 -0600
The utility of this is to reindex N partitions in 2 transactions, rather than
2*N transactions.
---
src/backend/commands/indexcmds.c | 258 ++++++++++++++-------
src/test/regress/expected/create_index.out | 4 +-
2 files changed, 182 insertions(+), 80 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index b21555b6cf..f5fea14ff4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -102,6 +102,8 @@ static void ReindexMultipleInternal(List *relids,
ReindexParams *params);
static bool ReindexRelationConcurrently(Oid relationOid,
ReindexParams *params);
+static List *ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
+ ReindexParams *params, MemoryContext private_context);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
@@ -124,6 +126,15 @@ typedef struct ReindexErrorInfo
char relkind;
} ReindexErrorInfo;
+/* Argument to ReindexIndexConcurrently takes a List* of these */
+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
@@ -2636,7 +2647,15 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
ReindexPartitions(indOid, params, isTopLevel);
else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
persistence != RELPERSISTENCE_TEMP)
- ReindexRelationConcurrently(indOid, params);
+ {
+ ReindexIndexInfo idxinfo = {
+ .indexId = indOid,
+ /* other fields set later */
+ };
+ ReindexIndexesConcurrently(list_make1(&idxinfo),
+ list_make1_oid(IndexGetRelation(indOid, false)),
+ params, CurrentMemoryContext);
+ }
else
{
ReindexParams newparams = *params;
@@ -3002,20 +3021,69 @@ 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_indexes(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);
+
+ /* Skip invalid indexes, if requested */
+ if ((options & REINDEXOPT_SKIPVALID) != 0 &&
+ get_index_isvalid(partoid))
+ continue;
+
+ /* (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
*
* Reindex a set of partitions, per the partitioned index or table given
* by the caller.
+ * XXX: should be further refactored with logic from ReindexRelationConcurrently
*/
static void
ReindexPartitions(Oid relid, ReindexParams *params, 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;
@@ -3060,38 +3128,58 @@ ReindexPartitions(Oid relid, ReindexParams *params, 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;
- /* Skip invalid indexes, if requested */
- if ((params->options & REINDEXOPT_SKIPVALID) != 0 &&
- get_index_isvalid(partoid))
- continue;
+ if (relkind == RELKIND_PARTITIONED_INDEX)
+ {
+ old_context = MemoryContextSwitchTo(reindex_context);
+ partitions = leaf_indexes(inhoids, params->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_indexes(partindexes, params->options, &heaprels);
+ partitions = list_concat(partitions, partindexes);
+
+ MemoryContextSwitchTo(old_context);
+ table_close(parttable, ShareLock);
+ }
+ }
- Assert(partkind == RELKIND_INDEX ||
- partkind == RELKIND_RELATION);
+ if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ relkind == RELKIND_PARTITIONED_INDEX &&
+ get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+ {
+ List *idxinfos = NIL;
+ ReindexIndexInfo *idxinfo;
- /* Save partition OID */
old_context = MemoryContextSwitchTo(reindex_context);
- partitions = lappend_oid(partitions, partoid);
+ foreach (lc, partitions)
+ {
+ Oid partoid = lfirst_oid(lc);
+ idxinfo = palloc(sizeof(ReindexIndexInfo));
+ idxinfo->indexId = partoid;
+ /* other fields set later */
+ idxinfos = lappend(idxinfos, idxinfo);
+ }
MemoryContextSwitchTo(old_context);
- }
- /*
- * Process each partition listed in a separate transaction. Note that
- * this commits and then starts a new transaction immediately.
- */
- ReindexMultipleInternal(partitions, params);
+ /* Process all indexes in a single loop */
+ ReindexIndexesConcurrently(idxinfos, heaprels, params, reindex_context);
+ } else {
+ /*
+ * Process each partition listed in a separate transaction. Note that
+ * this commits and then starts a new transaction immediately.
+ */
+ ReindexMultipleInternal(partitions, params);
+ }
/*
* If indexes exist on all of the partitioned table's children, and we
@@ -3254,18 +3342,9 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
static bool
ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
{
- typedef struct ReindexIndexInfo
- {
- Oid indexId;
- Oid tableId;
- Oid amId;
- bool safe; /* for set_indexsafe_procflags */
- } ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
List *newIndexIds = NIL;
- List *relationLocks = NIL;
- List *lockTags = NIL;
ListCell *lc,
*lc2;
MemoryContext private_context;
@@ -3274,13 +3353,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
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
@@ -3553,6 +3625,69 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
Assert(heapRelationIds != NIL);
+ /* Do the work */
+ newIndexIds = ReindexIndexesConcurrently(indexIds, heapRelationIds, params, private_context);
+
+ /* Log what we did */
+ if ((params->options & REINDEXOPT_VERBOSE) != 0)
+ {
+ 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 arbitrary list of index relations
+ * This is called by ReindexRelationConcurrently and
+ */
+static List *
+ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
+ ReindexParams *params, MemoryContext private_context)
+{
+ 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];
+
/*-----
* Now we have all the indexes we want to process in indexIds.
*
@@ -4026,42 +4161,9 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
/* Start a new transaction to finish process properly */
StartTransactionCommand();
- /* Log what we did */
- if ((params->options & REINDEXOPT_VERBOSE) != 0)
- {
- if (relkind == RELKIND_INDEX)
- ereport(INFO,
- (errmsg("index \"%s.%s\" was reindexed",
- relationNamespace, relationName),
- errdetail("%s.",
- pg_rusage_show(&ru0))));
- else
- {
- foreach(lc, newIndexIds)
- {
- ReindexIndexInfo *idx = lfirst(lc);
- Oid indOid = idx->indexId;
-
- 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;
}
/*
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 830fdddf24..6f41adf736 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2470,12 +2470,12 @@ COMMIT;
REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
-ERROR: cannot reindex system catalogs concurrently
+ERROR: concurrent index creation on system catalog tables is not supported
-- These are the toast table and index of pg_authid.
REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
-ERROR: cannot reindex system catalogs concurrently
+ERROR: concurrent index creation on system catalog tables is not supported
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
ERROR: cannot reindex system catalogs concurrently
-- Warns about catalog relations
--
2.17.0
--QTprm0S8XgL7H0Dt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v13-0006-More-refactoring.patch"
view thread (157+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v13 05/18] Refactor to allow reindexing all index partitions at once..
In-Reply-To: <no-message-id-348482@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox