From b04d46958b16bc07ecd6c2f07220599ecb304b4d Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 15 Feb 2021 14:12:32 -0600
Subject: [PATCH] WIP: progress reporting for CREATE INDEX on partitioned
 tables

See also:
a6642b3ae060976b42830b7dc8f29ec190ab05e4
03f9e5cba0ee1633af4abe734504df50af46fbd8
c880096dc1e14b62610aa34bc98db226fa134260
e66bcfb4c66ebc97020b1f7484b1529bd7993f23
---
 src/backend/catalog/index.c      |   6 +-
 src/backend/commands/indexcmds.c | 104 ++++++++++++++++++++++---------
 2 files changed, 77 insertions(+), 33 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f5441ce24a..fef9c831ac 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3733,9 +3733,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	 */
 	iRel = index_open(indexId, AccessExclusiveLock);
 
-	if (progress)
-		pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
-									 iRel->rd_rel->relam);
+	// Do this unconditionally?
+	pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
+								 iRel->rd_rel->relam);
 
 	/*
 	 * Partitioned indexes should never get processed here, as they have no
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 127ba7835d..8fdfeffce5 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -98,9 +98,10 @@ static void reindex_error_callback(void *args);
 static void ReindexPartitions(Oid relid, ReindexParams *params,
 							  bool isTopLevel);
 static void ReindexMultipleInternal(List *relids,
-									ReindexParams *params);
+									ReindexParams *params, bool ispartitioned);
 static bool ReindexRelationConcurrently(Oid relationOid,
-										ReindexParams *params);
+										ReindexParams *params,
+										bool ispartitioned);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
@@ -2600,7 +2601,7 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
 		ReindexPartitions(indOid, params, isTopLevel);
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 persistence != RELPERSISTENCE_TEMP)
-		ReindexRelationConcurrently(indOid, params);
+		ReindexRelationConcurrently(indOid, params, false);
 	else
 	{
 		ReindexParams newparams = *params;
@@ -2710,7 +2711,7 @@ ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel)
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
-		result = ReindexRelationConcurrently(heapOid, params);
+		result = ReindexRelationConcurrently(heapOid, params, false);
 
 		if (!result)
 			ereport(NOTICE,
@@ -2942,7 +2943,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 	 * Process each relation listed in a separate transaction.  Note that this
 	 * commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(relids, params);
+	ReindexMultipleInternal(relids, params, false);
 
 	MemoryContextDelete(private_context);
 }
@@ -3046,11 +3047,38 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 		MemoryContextSwitchTo(old_context);
 	}
 
+	/* progress reporting for partitioned indexes */
+	if (relkind == RELKIND_PARTITIONED_INDEX)
+	{
+		const int       progress_index[3] = {
+			PROGRESS_CREATEIDX_COMMAND,
+			PROGRESS_CREATEIDX_INDEX_OID,
+			PROGRESS_CREATEIDX_PARTITIONS_TOTAL
+		};
+
+		int64           progress_val[3] = {
+			(params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
+				get_rel_persistence(relid) != RELPERSISTENCE_TEMP ?
+				PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY :
+				PROGRESS_CREATEIDX_COMMAND_REINDEX,
+			relid,
+			list_length(partitions)
+		};
+
+		Oid tableid = IndexGetRelation(relid, false);
+		pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, tableid);
+
+		pgstat_progress_update_multi_param(3, progress_index, progress_val);
+	}
+
 	/*
 	 * Process each partition listed in a separate transaction.  Note that
 	 * this commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(partitions, params);
+	ReindexMultipleInternal(partitions, params, true);
+
+	if (relkind == RELKIND_PARTITIONED_INDEX)
+		pgstat_progress_end_command();
 
 	/*
 	 * Clean up working storage --- note we must do this after
@@ -3066,11 +3094,13 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
  * Reindex a list of relations, each one being processed in its own
  * transaction.  This commits the existing transaction immediately,
  * and starts a new transaction when finished.
+ * If isparititioned is true, then also handle progress reporting.
  */
 static void
-ReindexMultipleInternal(List *relids, ReindexParams *params)
+ReindexMultipleInternal(List *relids, ReindexParams *params, bool ispartitioned)
 {
 	ListCell   *l;
+	off_t		childnum = 0;
 
 	PopActiveSnapshot();
 	CommitTransactionCommand();
@@ -3128,15 +3158,16 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 			ReindexParams newparams = *params;
 
 			newparams.options |= REINDEXOPT_MISSING_OK;
-			(void) ReindexRelationConcurrently(relid, &newparams);
+			(void) ReindexRelationConcurrently(relid, &newparams, ispartitioned);
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
 		else if (relkind == RELKIND_INDEX)
 		{
 			ReindexParams newparams = *params;
 
-			newparams.options |=
-				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
+			/* Do not overwrite progress of partitioned tables */
+			newparams.options |= REINDEXOPT_MISSING_OK |
+				(ispartitioned ? 0 : REINDEXOPT_REPORT_PROGRESS);
 			reindex_index(relid, false, relpersistence, &newparams);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
@@ -3162,6 +3193,10 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 			PopActiveSnapshot();
 		}
 
+		if (ispartitioned)
+			pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+					childnum++);
+
 		CommitTransactionCommand();
 	}
 
@@ -3177,7 +3212,8 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
  * 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.
+ * itself will be rebuilt.  If "ispartitioned", then avoid overwriting progress
+ * of concrrently reindex of a partitioned index.
  *
  * 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
@@ -3193,7 +3229,8 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
  * anyway, and a non-concurrent reindex is more efficient.
  */
 static bool
-ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
+ReindexRelationConcurrently(Oid relationOid, ReindexParams *params,
+		bool ispartitioned)
 {
 	typedef struct ReindexIndexInfo
 	{
@@ -3215,13 +3252,13 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 	char	   *relationName = NULL;
 	char	   *relationNamespace = NULL;
 	PGRUsage	ru0;
+	int64		progress_vals[4];
 	const int	progress_index[] = {
 		PROGRESS_CREATEIDX_COMMAND,
 		PROGRESS_CREATEIDX_PHASE,
-		PROGRESS_CREATEIDX_INDEX_OID,
-		PROGRESS_CREATEIDX_ACCESS_METHOD_OID
+		PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
+		PROGRESS_CREATEIDX_INDEX_OID
 	};
-	int64		progress_vals[4];
 
 	/*
 	 * Create a memory context that will survive forced transaction commits we
@@ -3545,14 +3582,16 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 		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 (!ispartitioned)
+			pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+										  idx->tableId);
 
 		progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
 		progress_vals[1] = 0;	/* initializing */
-		progress_vals[2] = idx->indexId;
-		progress_vals[3] = idx->amId;
-		pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+		progress_vals[2] = idx->amId;
+		progress_vals[3] = idx->indexId;
+		pgstat_progress_update_multi_param(ispartitioned ? 3 : 4, progress_index,
+				progress_vals);
 
 		/* Choose a temporary relation name for the new index */
 		concurrentName = ChooseRelationName(get_rel_name(idx->indexId),
@@ -3700,12 +3739,14 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 		 * Update progress for the index to build, with the correct parent
 		 * table involved.
 		 */
-		pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, newidx->tableId);
+		if (!ispartitioned)
+			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] = newidx->indexId;
-		progress_vals[3] = newidx->amId;
-		pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+		progress_vals[2] = newidx->amId;
+		progress_vals[3] = newidx->indexId;
+		pgstat_progress_update_multi_param(ispartitioned ? 3 : 4, progress_index,
+				progress_vals);
 
 		/* Perform concurrent build of new index */
 		index_concurrently_build(newidx->tableId, newidx->indexId);
@@ -3764,13 +3805,15 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 		 * Update progress for the index to build, with the correct parent
 		 * table involved.
 		 */
-		pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
-									  newidx->tableId);
+		if (!ispartitioned)
+			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] = newidx->indexId;
-		progress_vals[3] = newidx->amId;
-		pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+		progress_vals[2] = newidx->amId;
+		progress_vals[3] = newidx->indexId;
+		pgstat_progress_update_multi_param(ispartitioned ? 3 : 4, progress_index,
+				progress_vals);
 
 		validate_index(newidx->tableId, newidx->indexId, snapshot);
 
@@ -4000,7 +4043,8 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 
 	MemoryContextDelete(private_context);
 
-	pgstat_progress_end_command();
+	if (!ispartitioned)
+		pgstat_progress_end_command();
 
 	return true;
 }
-- 
2.17.0

