public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ilya Gladyshev <[email protected]>
To: Justin Pryzby <[email protected]>
To: Alexander Pyhalov <[email protected]>
Cc: [email protected]
Cc: Masahiko Sawada <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: 李杰(慎追) <[email protected]>
Cc: 曾文旌(义从) <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Subject: Re: CREATE INDEX CONCURRENTLY on partitioned index
Date: Sat, 03 Dec 2022 19:13:30 +0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CAD21AoCHYE5=PU7kJGGUHZuYVrHTHqmpUxN+8-rLGYpdgPwJ9g@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
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
Attachments:
[text/x-patch] 0001-turn-off-reindex-reporting-for-create.patch (3.4K, ../[email protected]/2-0001-turn-off-reindex-reporting-for-create.patch)
download | inline diff:
From 8eb9fd7ce7d34c5c323c47b60a7f883f360ef090 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <[email protected]>
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
view thread (28+ 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], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: CREATE INDEX CONCURRENTLY on partitioned index
In-Reply-To: <[email protected]>
* 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