Hi,
I have noticed that progress reporting for CREATE INDEX of partitioned
tables seems to be working poorly for nested partitioned tables. In
particular, it overwrites total and done partitions count when it
recurses down to child partitioned tables and it only reports top-level
partitions. So it's not hard to see something like this during CREATE
INDEX now:
postgres=# select partitions_total, partitions_done from
pg_stat_progress_create_index ;
partitions_total | partitions_done
------------------+-----------------
1 | 2
(1 row)
I changed current behaviour to report the total number of partitions in
the inheritance tree and fixed recursion in the attached patch. I used
a static variable to keep the counter to avoid ABI breakage of
DefineIndex, so that we could backpatch this to previous versions.
Thanks,
Ilya Gladyshev
From 342caa3d4ce273b14a6998c20c32b862d2d4c890 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <ilya.v.gladys...@gmail.com>
Date: Fri, 9 Dec 2022 23:17:29 +0400
Subject: [PATCH] report top parent progress for CREATE INDEX
---
src/backend/commands/indexcmds.c | 36 ++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index b5b860c3ab..80557dad8d 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -129,6 +129,12 @@ typedef struct ReindexErrorInfo
char relkind;
} ReindexErrorInfo;
+/*
+ * Used to report the number of partitions,
+ * that were processed during index creation.
+ */
+static int create_index_parts_done;
+
/*
* CheckIndexCompatible
* Determine whether an existing index definition is compatible with a
@@ -1218,8 +1224,18 @@ DefineIndex(Oid relationId,
Relation parentIndex;
TupleDesc parentDesc;
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
- nparts);
+ if (!OidIsValid(parentIndexId))
+ {
+ List *inhs;
+
+ /* Reset counter of done partitions when processing root index */
+ create_index_parts_done = 0;
+ inhs = find_all_inheritors(relationId, NoLock, NULL);
+ /* inheritance tree size without root itself */
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
+ list_length(inhs) - 1);
+ list_free(inhs);
+ }
/* Make a local copy of partdesc->oids[], just for safety */
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
@@ -1431,8 +1447,6 @@ DefineIndex(Oid relationId,
child_save_sec_context);
}
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- i + 1);
free_attrmap(attmap);
}
@@ -1465,13 +1479,17 @@ DefineIndex(Oid relationId,
/*
* Indexes on partitioned tables are not themselves built, so we're
- * done here.
+ * done here. If it is a child partitioned table, increment done parts counter.
*/
AtEOXact_GUC(false, root_save_nestlevel);
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
+ else
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ ++create_index_parts_done);
+
return address;
}
@@ -1483,9 +1501,15 @@ DefineIndex(Oid relationId,
/* Close the heap and we're done, in the non-concurrent case */
table_close(rel, NoLock);
- /* If this is the top-level index, we're done. */
+ /*
+ * If this is the top-level index, we're done, otherwise, increment
+ * done partition counter.
+ */
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
+ else
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ ++create_index_parts_done);
return address;
}
--
2.30.2
