Re: [HACKERS] GUC for cleanup indexes threshold.

On Wed, Jun 20, 2018 at 1:00 AM, Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> 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.

Agreed.

>
>> >> 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.

Agreed. So should we move it to 19.11 Client Connection Defaults in
doc as well? And I think it's better if this option places next to
other vacuum options for finding easier. Attached patch changes them
so. Please review it.

> 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.

+1

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center