Re: Removing vacuum_cleanup_index_scale_factor

On Mon, Mar 8, 2021 at 10:21 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> Thank you for the patches. I looked at 0001 patch and have a comment:
>
> + * We don't report to the stats collector here because the stats collector
> + * only tracks per-table stats. Reset the changes_since_analyze counter
> + * only if we analyzed all columns; otherwise, there is still work for
> + * auto-analyze to do.
>
> I think the comment becomes clearer if we add "if doing inherited
> stats" at top of the above paragraph since we actually report to the
> stats collector in !inh case.

I messed the comment up. Oops. Fixed now.

> > Also included is a patch that removes the
> > vacuum_cleanup_index_scale_factor mechanism for triggering an index
> > scan during VACUUM -- that's what the second patch does (this depends
> > on the first patch, really).
>
> 0002 patch looks good to me.

Great.

Attached revision has a bit more polish. It includes new commit
messages which explains what we're really trying to fix here. I also
included backpatchable versions for Postgres 13 -- that's the other
significant change compared to the last version.

My current plan is to commit everything within the next day or two.
This includes backpatching to Postgres 13 only. I am now leaning
against doing anything in Postgres 11 and 12, for the closely related
btm_last_cleanup_num_heap_tuples VACUUM accuracy issue. There have
been no complaints from users using Postgres 11 or 12, so I'll leave
them alone. (Sorry for changing my mind again and again.)

To be clear: I plan on disabling (though not removing) the
vacuum_cleanup_index_scale_factor GUC and storage parameter on
Postgres 13, even though that is a stable release. This approach is
unorthodox, but it has a kind of a precedent -- the recheck_on_update
storage param was disabled on the Postgres 11 branch by commit
5d28c9bd. More importantly, it just happens to make sense, given the
specifics here.

--
Peter Geoghegan