On Wed, Jun 20, 2018 at 1:00 AM, Alexander Korotkov <a.korot...@postgrespro.ru> wrote: > On Tue, Jun 19, 2018 at 12:25 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: >> On Tue, Jun 19, 2018 at 5:43 PM, Alexander Korotkov >> <a.korot...@postgrespro.ru> wrote: >> > On Tue, Jun 19, 2018 at 11:34 AM Masahiko Sawada <sawada.m...@gmail.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
skip_cleanup_index_3.patch
Description: Binary data
