public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: David Rowley <[email protected]>
Cc: Sami Imseih <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Jeremy Schneider <[email protected]>
Cc: [email protected]
Subject: Re: another autovacuum scheduling thread
Date: Tue, 28 Oct 2025 16:03:23 -0500
Message-ID: <aQEvm40W3aVizp5Q@nathan> (raw)
In-Reply-To: <CAApHDvpOq09uVq7aXcuSBPAhZBTfAL-m2c4FOF2PphFe-YcnRg@mail.gmail.com>
References: <CAA5RZ0vSPqd5vP4-17E6QELRgQzaoKChgp5TDPK9GhZEK=0Gjg@mail.gmail.com>
	<aPp4VyLo2Zqk7oCV@nathan>
	<CAA5RZ0sfQ-VSCSafsrvyJ7wsW1utLwtPVJ5N6hB0726BGRDrgQ@mail.gmail.com>
	<CAApHDvpxE8ci83d02dRE3-fMetb4Dc89-80FrjkGDz2q+ByJog@mail.gmail.com>
	<CAA5RZ0upTpKqgrdNfMSX7UJdjx=+=CsQ6Xct+vcCZPvUVhdZvw@mail.gmail.com>
	<CAApHDvp1=FOs6GneTzLSCHnCmC7z1_80=U3M=CKd82-pwS3YHg@mail.gmail.com>
	<aPuWev3D9M4iGCUt@nathan>
	<CAApHDvoM5MEHHBc0TNdrzkpq39WdEHSZhdWrtnx9zOWNXTSFGw@mail.gmail.com>
	<aP-YgrcPi0EhgR9x@nathan>
	<CAApHDvpOq09uVq7aXcuSBPAhZBTfAL-m2c4FOF2PphFe-YcnRg@mail.gmail.com>

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 <[email protected]> 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


view thread (143+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: another autovacuum scheduling thread
  In-Reply-To: <aQEvm40W3aVizp5Q@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox