Re: CREATE INDEX CONCURRENTLY on partitioned index - Mailing list pgsql-hackers

On Mon, Feb 15, 2021 at 10:06:47PM +0300, Anastasia Lubennikova wrote:
> On 28.01.2021 17:30, Justin Pryzby wrote:
> > On Thu, Jan 28, 2021 at 09:51:51PM +0900, Masahiko Sawada wrote:
> > > On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > > On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
> > > > > Forking this thread, since the existing CFs have been closed.
> > > > > https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594b0f0ba40996afd
> > > > >
> > > > > The strategy is to create catalog entries for all tables with indisvalid=false,
> > > > > and then process them like REINDEX CONCURRENTLY.  If it's interrupted, it
> > > > > leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
> > > > > CIC on a plain table.
> > > > >
> > > > > On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> > > > > > On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
> > > > > > Note that the mentioned problem wasn't serious: there was missing index on
> > > > > > child table, therefor the parent index was invalid, as intended.  However I
> > > > > > agree that it's not nice that the command can fail so easily and leave behind
> > > > > > some indexes created successfully and some failed some not created at all.
> > > > > >
> > > > > > But I took your advice initially creating invalid inds.
> > > > > ...
> > > > > > That gave me the idea to layer CIC on top of Reindex, since I think it does
> > > > > > exactly what's needed.
> > > > > On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> > > > > > On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
> > > > > > > It would be good also to check if
> > > > > > > we have a partition index tree that maps partially with a partition
> > > > > > > table tree (aka no all table partitions have a partition index), where
> > > > > > > these don't get clustered because there is no index to work on.
> > > > > > This should not happen, since a incomplete partitioned index is "invalid".
>
> > > > > > I had been waiting to rebase since there hasn't been any review comments and I
> > > > > > expected additional, future conflicts.
> > > > > >
>
> I attempted to review this feature, but the last patch conflicts with the
> recent refactoring, so I wasn't able to test it properly.
> Could you please send a new version?

I rebased this yesterday, so here's my latest.

> 2) Here we access relation field after closing the relation. Is it safe?
>     /* save lockrelid and locktag for below */
>     heaprelid = rel->rd_lockInfo.lockRelId;

Thanks, fixed this just now.

> 3) leaf_partitions() function only handles indexes, so I suggest to name it
> more specifically and add a comment about meaning of 'options' parameter.
>
> 4) I don't quite understand the idea of the regression test. Why do we
> expect to see invalid indexes there?
> +    "idxpart_a_idx1" UNIQUE, btree (a) INVALID

Because of the unique failure:
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+ERROR:  could not create unique index "idxpart2_a_idx2_ccnew"
+DETAIL:  Key (a)=(10) is duplicated.
+\d idxpart

This shows that CIC first creates catalog-only INVALID indexes, and then
reindexes them to "validate".

--
Justin