Re: Berserk Autovacuum (let's save next Mandrill)

On Fri, 6 Mar 2020 at 23:46, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> wrote:
>
> Thanks, Justin, for the review.
> I have applied the changes where still applicable.
>
> On Fri, 2020-03-06 at 10:52 +1300, David Rowley wrote:
> > On Fri, 6 Mar 2020 at 03:27, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> wrote:
> > > On Thu, 2020-03-05 at 19:40 +1300, David Rowley wrote:
> > > > 1. I'd go for 2 new GUCs and reloptions.
> > > > autovacuum_vacuum_insert_threshold (you're currently calling this
> > > > autovacuum_vacuum_insert_limit. I don't see why the word "limit" is
> > > > relevant here). The other GUC I think should be named
> > > > autovacuum_vacuum_insert_scale_factor and these should work exactly
> > > > the same way as autovacuum_vacuum_threshold and
> > > > autovacuum_vacuum_scale_factor, but be applied in a similar way to the
> > > > vacuum settings, but only be applied after we've checked to ensure the
> > > > table is not otherwise eligible to be vacuumed.
> > >
> > > I disagree about the scale_factor (and have not added it to the
> > > updated version of the patch). If we have a scale_factor, then the
> > > time between successive autovacuum runs would increase as the table
> > > gets bigger, which defeats the purpose of reducing the impact of each
> > > autovacuum run.
> >
> > My view here is not really to debate what logically makes the most
> > sense. I don't really think for a minute that the current
> > auto-vacuums scale_factor and thresholds are perfect for the job. It's
> > true that the larger a table becomes, the less often it'll be
> > vacuumed, but these are control knobs that people have become
> > accustomed to and I don't really think that making an exception for
> > this is warranted. Perhaps we can zero out the scale factor by
> > default and set the threshold into the millions of tuples. We can have
> > people chime in on what they think about that and why once the code is
> > written and even perhaps committed.
>
> Ok, I submit. My main desire was to keep the number of new GUCs as
> low as reasonably possible, but making the feature tunable along the
> known and "trusted" lines may be a good thing.
>
> The new parameter is called "autovacuum_vacuum_insert_scale_factor".
>
> > Lack of a scale_factor does leave people who regularly truncate their
> > "append-only" tables out in the cold a bit. Perhaps they'd like
> > index-only scans to kick in soon after they truncate without having to
> > wait for 10 million tuples, or so.
>
> That point I don't see.
> Truncating a table resets the counters to 0.
>
> > > > 10. I'm slightly worried about the case where we don't quite trigger a
> > > > normal vacuum but trigger a vacuum due to INSERTs then skip cleaning
> > > > up the indexes but proceed to leave dead index entries causing indexes
> > > > to become bloated. It does not seem impossible that given the right
> > > > balance of INSERTs and UPDATE/DELETEs that this could happen every
> > > > time and the indexes would just become larger and larger.
> > >
> > > Perhaps we can take care of the problem by *not* skipping index
> > > cleanup if "changes_since_analyze" is substantially greater than 0.
> > >
> > > What do you think?
> >
> > Well, there is code that skips the index scans when there are 0 dead
> > tuples found in the heap. If the table is truly INSERT-only then it
> > won't do any harm since we'll skip the index scan anyway. I think
> > it's less risky to clean the indexes. If we skip that then there will
> > be a group of people will suffer from index bloat due to this, no
> > matter if they realise it or not.

+1

FYI actually vacuum could perform index cleanup phase (i.g.
PROGRESS_VACUUM_PHASE_INDEX_CLEANUP phase) on a table even if it's a
truly INSERT-only table, depending on
vacuum_cleanup_index_scale_factor. Anyway, I also agree with not
disabling index cleanup in insert-only vacuum case, because it could
become not only a cause of index bloat but also a big performance
issue. For example, if autovacuum on a table always run without index
cleanup, gin index on that table will accumulate insertion tuples in
its pending list and will be cleaned up by a backend process while
inserting new tuple, not by a autovacuum process. We can disable index
vacuum by index_cleanup storage parameter per tables, so it would be
better to defer these settings to users.

I have one question about this patch from architectural perspective:
have you considered to use autovacuum_vacuum_threshold and
autovacuum_vacuum_scale_factor also for this purpose? That is, we
compare the threshold computed by these values to not only the number
of dead tuples but also the number of inserted tuples. If the number
of dead tuples exceeds the threshold, we trigger autovacuum as usual.
On the other hand if the number of inserted tuples exceeds, we trigger
autovacuum with vacuum_freeze_min_age = 0. I'm concerned that how user
consider the settings of newly added two parameters. We will have in
total 4 parameters. Amit also was concerned about that[1].

I think this idea also works fine. In insert-only table case, since
only the number of inserted tuples gets increased, only one threshold
(that is, threshold computed by autovacuum_vacuum_threshold and
autovacuum_vacuum_scale_factor) is enough to trigger autovacuum. And
in mostly-insert table case, in the first place, we can trigger
autovacuum even in current PostgreSQL, since we have some dead tuples.
But if we want to trigger autovacuum more frequently by the number of
newly inserted tuples, we can set that threshold lower while
considering only the number of inserted tuples.

And I briefly looked at this patch:

@@ -2889,7 +2898,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
tab->at_params.truncate = VACOPT_TERNARY_DEFAULT;
/* As of now, we don't support parallel vacuum for autovacuum */
tab->at_params.nworkers = -1;
- tab->at_params.freeze_min_age = freeze_min_age;
+ tab->at_params.freeze_min_age = freeze_all ? 0 : freeze_min_age;
tab->at_params.freeze_table_age = freeze_table_age;
tab->at_params.multixact_freeze_min_age = multixact_freeze_min_age;
tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age;

I think we can set multixact_freeze_min_age to 0 as well.

Regards,

[1] https://www.postgresql.org/message-id/CAA4eK1%2BrCxS_Pg4GdSa6G8ESOTHK%2BjDVgqYd_dnO07rGNaewKA%40mail.gmail.com

--
Masahiko Sawada http://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services