REINDEX backend filtering

>
> On Thu, Dec 03, 2020 at 05:31:43PM +0800, Julien Rouhaud wrote:
> > Now that we have the infrastructure to track indexes that might be corrupted
> > due to changes in collation libraries, I think it would be a good idea to offer
> > an easy way for users to reindex all indexes that might be corrupted.
>
> Yes.  It would be a good thing.
>
> > The filter is also implemented so that you could cumulate multiple filters, so
> > it could be easy to add more filtering, for instance:
> >
> > REINDEX (COLLATION 'libc', COLLATION 'not_current') DATABASE mydb;
> >
> > to only rebuild indexes depending on outdated libc collations, or
> >
> > REINDEX (COLLATION 'libc', VERSION 'X.Y') DATABASE mydb;
> >
> > to only rebuild indexes depending on a specific version of libc.
>
> Deciding on the grammar to use depends on the use cases we would like
> to satisfy.  From what I heard on this topic, the goal is to reduce
> the amount of time necessary to reindex a system so as REINDEX only
> works on indexes whose dependent collation versions are not known or
> works on indexes in need of a collation refresh (like a reindexdb
> --all --collation -j $jobs).  What would be the benefit in having more
> complexity with library-dependent settings while we could take care
> of the use cases that matter the most with a simple grammar?  Perhaps
> "not_current" is not the best match as a keyword, we could just use
> "collation" and handle that as a boolean.  As long as we don't need
> new operators in the grammar rules..

>
> On Mon, Dec 14, 2020 at 3:45 PM Michael Paquier <[hidden email]> wrote:
> >
> > On Thu, Dec 03, 2020 at 05:31:43PM +0800, Julien Rouhaud wrote:
> > > Now that we have the infrastructure to track indexes that might be corrupted
> > > due to changes in collation libraries, I think it would be a good idea to offer
> > > an easy way for users to reindex all indexes that might be corrupted.
> >
> > Yes.  It would be a good thing.
> >
> > > The filter is also implemented so that you could cumulate multiple filters, so
> > > it could be easy to add more filtering, for instance:
> > >
> > > REINDEX (COLLATION 'libc', COLLATION 'not_current') DATABASE mydb;
> > >
> > > to only rebuild indexes depending on outdated libc collations, or
> > >
> > > REINDEX (COLLATION 'libc', VERSION 'X.Y') DATABASE mydb;
> > >
> > > to only rebuild indexes depending on a specific version of libc.
> >
> > Deciding on the grammar to use depends on the use cases we would like
> > to satisfy.  From what I heard on this topic, the goal is to reduce
> > the amount of time necessary to reindex a system so as REINDEX only
> > works on indexes whose dependent collation versions are not known or
> > works on indexes in need of a collation refresh (like a reindexdb
> > --all --collation -j $jobs).  What would be the benefit in having more
> > complexity with library-dependent settings while we could take care
> > of the use cases that matter the most with a simple grammar?  Perhaps
> > "not_current" is not the best match as a keyword, we could just use
> > "collation" and handle that as a boolean.  As long as we don't need
> > new operators in the grammar rules..
>
> I'm not sure what the DBA usual pattern here.  If the reindexing
> runtime is really critical, I'm assuming that at least some people
> will dig into library details to see what are the collations that
> actually broke in the last upgrade and will want to reindex only
> those, and force the version for the rest of the indexes.  And
> obviously, they probably won't wait to have multiple collation
> versions dependencies before taking care of that.  In that case the
> filters that would matters would be one to only keep indexes with an
> outdated collation version, and an additional one for a specific
> collation name.  Or we could have the COLLATION keyword without
> additional argument mean all outdated collations, and COLLATION
> 'collation_name' to specify a specific one.  This is maybe a bit ugly,
> and would probably require a different approach for reindexdb.

>
> > The code as-is written to be extensible with possibly other filters
> > (e.g. specific library or specific version).  Feedback so far seems to
> > indicate that it may be overkill and only filtering indexes with
> > deprecated collation is enough.  I'll simplify this code in a future
> > version, getting rid of reindexHasFilter, unless someone thinks more
> > filter is a good idea.
>
> Hmm, yeah, I think it should probably be very general. Suppose someone
> invents versioned dependencies for (say) functions or full text
> stemmers etc, then do we want to add more syntax here to rebuild
> indexes (assuming we don't use INVALID for such cases, IDK)?  I don't
> think we'd want to have more syntax just to be able to say "hey,
> please fix my collation problems but not my stemmer problems".  What
> about just REINDEX (OUTDATED)?  It's hard to find a single word that
> means "depends on an object whose version has changed".

>
> On Tue, Dec 15, 2020 at 06:34:16PM +0100, Magnus Hagander wrote:
> > Is this really a common enough operation that we need it in the main grammar?
> >
> > Having the functionality, definitely, but what if it was "just" a
> > function instead? So you'd do something like:
> > SELECT 'reindex index ' || i FROM pg_blah(some, arguments, here)
> > \gexec
> >
> > Or even a function that returns the REINDEX commands directly (taking
> > a parameter to turn on/off concurrency for example).
> >
> > That also seems like it would be easier to make flexible, and just as
> > easy to plug into reindexdb?
>
> Having control in the grammar to choose which index to reindex for a
> table is very useful when it comes to parallel reindexing, because
> it is no-brainer in terms of knowing which index to distribute to one
> job or another.  In short, with this grammar you can just issue a set
> of REINDEX TABLE commands that we know will not conflict with each
> other.  You cannot get that level of control with REINDEX INDEX as it

>
> On Wed, Dec 16, 2020 at 8:27 AM Michael Paquier <[hidden email]> wrote:
> >
> > On Tue, Dec 15, 2020 at 06:34:16PM +0100, Magnus Hagander wrote:
> > > Is this really a common enough operation that we need it in the main grammar?
> > >
> > > Having the functionality, definitely, but what if it was "just" a
> > > function instead? So you'd do something like:
> > > SELECT 'reindex index ' || i FROM pg_blah(some, arguments, here)
> > > \gexec
> > >
> > > Or even a function that returns the REINDEX commands directly (taking
> > > a parameter to turn on/off concurrency for example).
> > >
> > > That also seems like it would be easier to make flexible, and just as
> > > easy to plug into reindexdb?
> >
> > Having control in the grammar to choose which index to reindex for a
> > table is very useful when it comes to parallel reindexing, because
> > it is no-brainer in terms of knowing which index to distribute to one
> > job or another.  In short, with this grammar you can just issue a set
> > of REINDEX TABLE commands that we know will not conflict with each
> > other.  You cannot get that level of control with REINDEX INDEX as it
> > may be possible that more or more commands conflict if they work on an
> > index of the same relation because it is required to take lock also on
> > the parent table.  Of course, we could decide to implement a
> > redistribution logic in all frontend tools that need such things, like
> > reindexdb, but that's not something I think we should let the client
> > decide of.  A backend-side filtering is IMO much simpler, less code,
> > and more elegant.
>
> Maybe additional filtering capabilities is not something that will be
> required frequently, but I'm pretty sure that reindexing only indexes
> that might be corrupt is something that will be required often..  So I
> agree, having all that logic in the backend makes everything easier
> for users, having the choice of the tools they want to issue the query
> while still having all features available.

>
> I don't agree with the conclusion though.
>
> The most common case of that will be in the case of an upgrade. In
> that case I want to reindex all of those indexes as quickly as
> possible. So I'll want to parallelize it across multiple sessions
> (like reindexdb -j 4 or whatever). But doesn't putting the filter in
> the grammar prevent me from doing exactly that? Each of those 4 (or
> whatever) sessions would have to guess which would go where and then
> speculatively run the command on that, instead of being able to
> directly distribute the worload?

>
> On Fri, Feb 26, 2021 at 5:50 PM Magnus Hagander <[hidden email]> wrote:
> >
> > I don't agree with the conclusion though.
> >
> > The most common case of that will be in the case of an upgrade. In
> > that case I want to reindex all of those indexes as quickly as
> > possible. So I'll want to parallelize it across multiple sessions
> > (like reindexdb -j 4 or whatever). But doesn't putting the filter in
> > the grammar prevent me from doing exactly that? Each of those 4 (or
> > whatever) sessions would have to guess which would go where and then
> > speculatively run the command on that, instead of being able to
> > directly distribute the worload?
>
> It means that you'll have to distribute the work on a per-table basis
> rather than a per-index basis.  The time spent to find out that a
> table doesn't have any impacted index should be negligible compared to
> the cost of running a reindex.  This obviously won't help that much if
> you have a lot of table but only one being gigantic.

> But even if we put the logic in the client, this still won't help as
> reindexdb doesn't support multiple job with an index list:
>
>          * Index-level REINDEX is not supported with multiple jobs as we
>          * cannot control the concurrent processing of multiple indexes
>          * depending on the same relation.
>          */
>         if (concurrentCons > 1 && indexes.head != NULL)
>         {
>             pg_log_error("cannot use multiple jobs to reindex indexes");
>             exit(1);
>         }

> On Fri, Feb 26, 2021 at 11:07 AM Julien Rouhaud <[hidden email]> wrote:
> >
> > It means that you'll have to distribute the work on a per-table basis
> > rather than a per-index basis.  The time spent to find out that a
> > table doesn't have any impacted index should be negligible compared to
> > the cost of running a reindex.  This obviously won't help that much if
> > you have a lot of table but only one being gigantic.
>
> Yeah -- or at least a couple of large and many small, which I find to
> be a very common scenario. Or the case of some tables having many
> affected indexes and some having few.
>
> You'd basically want to order the operation by table on something like
> "total size of the affected indexes on table x" -- which may very well
> put a smaller table with many indexes earlier in the queue. But you
> can't do that without having access to the filter....

> > But even if we put the logic in the client, this still won't help as
> > reindexdb doesn't support multiple job with an index list:
> >
> >          * Index-level REINDEX is not supported with multiple jobs as we
> >          * cannot control the concurrent processing of multiple indexes
> >          * depending on the same relation.
> >          */
> >         if (concurrentCons > 1 && indexes.head != NULL)
> >         {
> >             pg_log_error("cannot use multiple jobs to reindex indexes");
> >             exit(1);
> >         }
>
> That sounds like it would be a fixable problem though, in principle.
> It could/should probably still limit all indexes on the same table to
> be processed in the same connection for the locking reasons of course,
> but doing an order by the total size of the indexes like above, and
> ensuring that they are grouped that way, doesn't sound *that* hard. I
> doubt it's that important in the current usecase of manually listing
> the indexes, but it would be useful for something like this.