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

On Tue, Mar 17, 2020 at 01:14:02AM +0100, Laurenz Albe wrote:
> lazy_check_needs_freeze() is only called for an aggressive vacuum, which
> this isn't.

> --- a/src/backend/access/heap/vacuumlazy.c
> +++ b/src/backend/access/heap/vacuumlazy.c
> @@ -1388,17 +1388,26 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
>              else
>              {
>                  bool        tuple_totally_frozen;
> +                bool        freeze_all;
>  
>                  num_tuples += 1;
>                  hastup = true;
>  
> +                /*
> +                 * If any tuple was already frozen in the block and this is
> +                 * an insert-only vacuum, we might as well freeze all other
> +                 * tuples in that block.
> +                 */
> +                freeze_all = params->is_insert_only && has_dead_tuples;
> +

You're checking if any (previously-scanned) tuple was *dead*, but I think you
need to check nfrozen>=0.

Also, this will fail to freeze tuples on a page which *could* be
oppotunistically-frozen, but *follow* the first tuple which *needs* to be
frozen.

I think Andres was thinking this would maybe be an optimization independent of
is_insert_only (?)

>                  /*
>                   * Each non-removable tuple must be checked to see if it needs
>                   * freezing.  Note we already have exclusive buffer lock.
>                   */
>                  if (heap_prepare_freeze_tuple(tuple.t_data,
>                                                relfrozenxid, relminmxid,
> -                                              FreezeLimit, MultiXactCutoff,
> +                                              freeze_all ? 0 : FreezeLimit,
> +                                              freeze_all ? 0 : MultiXactCutoff,
>                                                &frozen[nfrozen],
>                                                &tuple_totally_frozen))

> +    /* normal autovacuum shouldn't freeze aggressively */
> +    *insert_only = false;

Aggressively is a bad choice of words.  In the context of vacuum, it usually
means "visit all pages, even those which are allvisible".

-- 
Justin