Re: another autovacuum scheduling thread

On Tue, Oct 28, 2025 at 11:47:08AM +1300, David Rowley wrote:
> 1. I think the following code at the bottom of
> relation_needs_vacanalyze() can be deleted. You've added the check to
> ensure *doanalyze never gets set to true for pg_statistic.
> 
> /* ANALYZE refuses to work with pg_statistic */
> if (relid == StatisticRelationId)
>     *doanalyze = false;
> 
> 2. As #1, but in recheck_relation_needs_vacanalyze(), the following I
> think can now be removed:
> 
> /* ignore ANALYZE for toast tables */
> if (classForm->relkind == RELKIND_TOASTVALUE)
>     *doanalyze = false;

Removed.

> 3. Would you be able to include what the idea behind the * 1.05 in the
> preceding comment?
> 
> On Tue, 28 Oct 2025 at 05:06, Nathan Bossart <nathandbossart@gmail.com> wrote:
>> +        effective_xid_failsafe_age = Max(vacuum_failsafe_age,
>> +                                         autovacuum_freeze_max_age * 1.05);
>> +        effective_mxid_failsafe_age = Max(vacuum_multixact_failsafe_age,
>> +                                          autovacuum_multixact_freeze_max_age * 1.05);
> 
> I assume it's to workaround some strange configuration settings, but
> don't know for sure, or why 1.05 is a good value.

This is lifted from vacuum_xid_failsafe_check().  As noted in the docs, the
failsafe settings are silently limited to 105% of *_freeze_max_age.  I
expanded on this in the comment atop these lines.

> 4. I think it might be neater to format the following as 3 separate "if" tests:
> 
>> +        if (force_vacuum ||
>> +            vactuples > vacthresh ||
>> +            (vac_ins_base_thresh >= 0 && instuples > vacinsthresh))
>> +        {
>> +            *dovacuum = true;
>> +            *score = Max(*score, (double) vactuples / Max(vacthresh, 1));
>> +            if (vac_ins_base_thresh >= 0)
>> +                *score = Max(*score, (double) instuples / Max(vacinsthresh, 1));
>> +        }
>> +        else
>> +            *dovacuum = false;
> 
> i.e:
> 
>         if (force_vacuum)
>             *dovacuum = true;
> 
>         if (vactuples > vacthresh)
>         {
>             *dovacuum = true;
>             *score = Max(*score, (double) vactuples / Max(vacthresh, 1));
>         }
> 
>         if (vac_ins_base_thresh >= 0 && instuples > vacinsthresh)
>         {
>             *dovacuum = true;
>             *score = Max(*score, (double) instuples / Max(vacinsthresh, 1));
>         }
> 
> and also get rid of all the "else *dovacuum = false;" (and *dovacuum =
> false) in favour of setting those to false at the top of the function.
> It's just getting harder to track that those parameters are getting
> set in all cases when they're meant to be.
> 
> doing that also gets rid of the duplicative "if (vac_ins_base_thresh
> >= 0)" check and also saves doing the score calc when the inputs to it
> don't make sense. The current code is relying on Max always picking
> the current *score when the threshold isn't met.

Done.

-- 
nathan