Hi,
Thank you Justin and Alexander for working on this, I have reviewed and
tested the latest patch, it works well, the problems mentioned
previously are all fixed. I like the idea of sharing code of reindex
and index, but I have noticed some peculiarities as a user.
The reporting is somewhat confusing as it switches to reporting for
reindex concurrently while building child indexes, this should be fixed
with the simple patch I have attached. Another thing that I have
noticed is that REINDEX, which is used under the hood, creates new
indexes with suffix _ccnew, and if the index building fails, the
indexes that could not be build will have the name with _ccnew suffix.
This can actually be seen in your test:
ERROR: could not create unique index "idxpart2_a_idx2_ccnew"
I find it quite confusing and I don't think that this the expected
behavior (if it is, I think it should be documented, like it is for
REINDEX). As an example of problems that it might entail, DROP INDEX
will not drop all the invalid indexes in the inheritance tree, because
it will leave _ccnew indexes in place, which is ok for reindex
concurrently, but that's not how C-I-C works now. I think that fixing
this problem requires some heavy code rewrite and I'm not quite sure
how to go about it, if you have any ideas, I will be happy to try them
out.
Thanks,
Ilya
From 8eb9fd7ce7d34c5c323c47b60a7f883f360ef090 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <ilya.v.gladys...@gmail.com>
Date: Sat, 3 Dec 2022 18:20:03 +0400
Subject: [PATCH] turn off reindex reporting for create
---
src/backend/commands/indexcmds.c | 36 +++++++++++++++++++-------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a2775931e2..b3c713037f 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3804,14 +3804,16 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
/* Don't overwrite CREATE INDEX command */
if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ {
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[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);
+ }
/* Choose a temporary relation name for the new index */
concurrentName = ChooseRelationName(get_rel_name(idx->indexId),
@@ -3967,13 +3969,15 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
* table involved. Don't overwrite CREATE INDEX command.
*/
if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ {
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[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);
+ }
/* Perform concurrent build of new index */
index_concurrently_build(newidx->tableId, newidx->indexId);
@@ -4033,14 +4037,16 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
* table involved. Don't overwrite CREATE INDEX command.
*/
if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ {
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[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);
+ }
validate_index(newidx->tableId, newidx->indexId, snapshot);
--
2.30.2
