Hi,
While working on CIC for partitioned tables [1], I noticed that REINDEX
for partitioned tables is not tracking keeping progress of partitioned
tables, so I'm creating a separate thread for this fix as suggested.
The way REINDEX for partitioned tables works now ReindexMultipleInternal
treats every partition as an independent table without keeping track of
partition_done/total counts, because this is just generic code for
processing multiple tables. The patch addresses that by passing down the
knowledge a flag to distinguish partitions from independent tables
in ReindexMultipleInternal and its callees. I also noticed that the
partitioned CREATE INDEX progress tracking could also benefit from
progress_index_partition_done function that zeroizes params in addition
to incrementing the counter, so I applied it there as well.
[1]
https://www.postgresql.org/message-id/55cfae76-2ffa-43ed-a7e7-901bffbebee4%40gmail.com
From 18baa028e1cc5c39347b9126ec1a96eb99e8e3e1 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <ilya.v.gladys...@gmail.com>
Date: Tue, 11 Jun 2024 17:48:08 +0100
Subject: [PATCH] make REINDEX track partition progress
---
src/backend/catalog/index.c | 11 ++++--
src/backend/commands/indexcmds.c | 61 +++++++++++++++++++++++++++++---
src/include/catalog/index.h | 1 +
3 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index a819b4197c..90488f9140 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3560,6 +3560,7 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
volatile bool skipped_constraint = false;
PGRUsage ru0;
bool progress = ((params->options & REINDEXOPT_REPORT_PROGRESS) != 0);
+ bool partition = ((params->options & REINDEXOPT_PARTITION) != 0);
bool set_tablespace = false;
pg_rusage_init(&ru0);
@@ -3605,8 +3606,9 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
indexId
};
- pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
- heapId);
+ if (!partition)
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ heapId);
pgstat_progress_update_multi_param(2, progress_cols, progress_vals);
}
@@ -3846,8 +3848,11 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
index_close(iRel, NoLock);
table_close(heapRelation, NoLock);
- if (progress)
+ if (progress && !partition)
+ {
+ /* progress for partitions is tracked in the caller */
pgstat_progress_end_command();
+ }
}
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 2caab88aa5..2196279108 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -113,6 +113,7 @@ static bool ReindexRelationConcurrently(const ReindexStmt *stmt,
const ReindexParams *params);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
+static inline void progress_index_partition_done(void);
/*
* callback argument type for RangeVarCallbackForReindexIndex()
@@ -1569,7 +1570,7 @@ DefineIndex(Oid tableId,
else
{
/* Update progress for an intermediate partitioned index itself */
- pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ progress_index_partition_done();
}
return address;
@@ -1590,7 +1591,7 @@ DefineIndex(Oid tableId,
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
else
- pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ progress_index_partition_done();
return address;
}
@@ -3213,6 +3214,14 @@ ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *param
ListCell *lc;
ErrorContextCallback errcallback;
ReindexErrorInfo errinfo;
+ ReindexParams newparams;
+ int progress_params[3] = {
+ PROGRESS_CREATEIDX_COMMAND,
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_PARTITIONS_TOTAL
+ };
+ int64 progress_values[3];
+ Oid heapId = relid;
Assert(RELKIND_HAS_PARTITIONS(relkind));
@@ -3274,11 +3283,28 @@ ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *param
MemoryContextSwitchTo(old_context);
}
+ if (relkind == RELKIND_PARTITIONED_INDEX)
+ {
+ heapId = IndexGetRelation(relid, true);
+ }
+
+ progress_values[0] = (params->options & REINDEXOPT_CONCURRENTLY) != 0 ?
+ PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY :
+ PROGRESS_CREATEIDX_COMMAND_REINDEX;
+ progress_values[1] = 0;
+ progress_values[2] = list_length(partitions);
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, heapId);
+ pgstat_progress_update_multi_param(3, progress_params, progress_values);
+
/*
* Process each partition listed in a separate transaction. Note that
* this commits and then starts a new transaction immediately.
*/
- ReindexMultipleInternal(stmt, partitions, params);
+ newparams = *params;
+ newparams.options |= REINDEXOPT_PARTITION;
+ ReindexMultipleInternal(stmt, partitions, &newparams);
+
+ pgstat_progress_end_command();
/*
* Clean up working storage --- note we must do this after
@@ -3299,6 +3325,7 @@ static void
ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const ReindexParams *params)
{
ListCell *l;
+ bool partitions = ((params->options & REINDEXOPT_PARTITION) != 0);
PopActiveSnapshot();
CommitTransactionCommand();
@@ -3392,6 +3419,9 @@ ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const Reind
}
CommitTransactionCommand();
+
+ if (partitions)
+ progress_index_partition_done();
}
StartTransactionCommand();
@@ -3444,6 +3474,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
char *relationName = NULL;
char *relationNamespace = NULL;
PGRUsage ru0;
+ bool partition = ((params->options & REINDEXOPT_PARTITION) != 0);
const int progress_index[] = {
PROGRESS_CREATEIDX_COMMAND,
PROGRESS_CREATEIDX_PHASE,
@@ -3787,7 +3818,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
elog(ERROR, "cannot reindex a temporary table concurrently");
- pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, idx->tableId);
+ if (!partition)
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, idx->tableId);
progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
progress_vals[1] = 0; /* initializing */
@@ -4261,7 +4293,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
MemoryContextDelete(private_context);
- pgstat_progress_end_command();
+ if (!partition)
+ pgstat_progress_end_command();
return true;
}
@@ -4454,3 +4487,21 @@ set_indexsafe_procflags(void)
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
}
+
+static inline void
+progress_index_partition_done(void)
+{
+ int nparam = 6;
+ const int progress_idx[] = {
+ PROGRESS_CREATEIDX_INDEX_OID,
+ PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_SUBPHASE,
+ PROGRESS_CREATEIDX_TUPLES_TOTAL,
+ PROGRESS_CREATEIDX_TUPLES_DONE
+ };
+ const int64 progress_vals[] = {0, 0, 0, 0, 0, 0};
+
+ pgstat_progress_update_multi_param(nparam, progress_idx, progress_vals);
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+}
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 7d434f8e65..ccba65fbbf 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -42,6 +42,7 @@ typedef struct ReindexParams
#define REINDEXOPT_REPORT_PROGRESS 0x02 /* report pgstat progress */
#define REINDEXOPT_MISSING_OK 0x04 /* skip missing relations */
#define REINDEXOPT_CONCURRENTLY 0x08 /* concurrent mode */
+#define REINDEXOPT_PARTITION 0x10 /* reindexing is done as part of partitioned table/index reindex */
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
--
2.43.0
