Re: [HACKERS] GUC for cleanup indexes threshold.

On Tue, Jun 19, 2018 at 12:25 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Tue, Jun 19, 2018 at 5:43 PM, Alexander Korotkov
> <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> > On Tue, Jun 19, 2018 at 11:34 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >> On Mon, Jun 18, 2018 at 1:56 PM, Alexander Korotkov
> >> > So, I'm proposing to raise maximum valus of
> >> > vacuum_cleanup_index_scale_factor to DBL_MAX. Any objections?
> >> >
> >>
> >> I agree to expand the maximum value. But if users don't want index
> >> cleanup it would be helpful if we have an option (e.g. setting to -1)
> >> to disable index cleanup while documenting a risk of disabling index
> >> cleanup. It seems to me that setting very high values means the same
> >> purpose.
> >
> > Yes, providing an option to completely disable b-tree index cleanup
> > would be good. But the problem is that we already use -1 value for
> > "use the default" in reloption. So, if even we will make -1 guc
> > option to mean "never cleanup", then we still wouldn't be able to make
> > reloption to work this way. Probably, we should use another "magical
> > value" in reloption for "use the default" semantics.
>
> Right. We can add a new reloption specifying whether we use default
> value of vacuum_index_cleanup_scale_factor or not but it might be
> overkill.

That would be overkill, and also that would be different from how
other reloptions behaves.

> >> Also, your patch lacks documentation update.
> >
> > Good catch, thank you.
> >
> >> BTW, I realized that postgresql.conf.sample doesn't have
> >> vacuum_cleanup_index_scale_factor option. Attached patch fixes it.
> >
> > It seems that you post a wrong attachment, because the patch you sent
> > is exactly same as mine.
> >
>
> Sorry, attached correct one.

Ok. I've rephrased comment a bit. Also, you created "index vacuum"
subsection in the "resource usage" section. I think it's not
appropriate for this option to be in "resource usage". Ideally it
should be grouped with other vacuum options, but we don't have single
section for that. "Autovacuum" section is also not appropriate,
because this guc works not only for autovacuum. So, most semantically
close options, which affects vacuum in general, are
vacuum_freeze_min_age, vacuum_freeze_table_age,
vacuum_multixact_freeze_min_age and vacuum_multixact_freeze_table_age,
which are located in "client connection defaults" section. So, I
decided to move this GUC into this section. I also change the section
in GUC definition (guc.c) from AUTOVACUUM to CLIENT_CONN_STATEMENT.

I think we ideally should have a "vacuum" section, which would have
two subections: "client defaults" and "autovacuum". But that should
be a separate patch, which needs to be discussed separately.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company