MIME-Version: 1.0
References: <aPklC5V61VcTu7IP@nathan> <aPkpSXHB66islP3h@nathan>
 <CAApHDvqobtKMwJbhKB_c=3-TM=TgS3bcuvzcWMm3ee1c0mz9hw@mail.gmail.com>
 <aPkz9grlBAJRXrbe@nathan>
 <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>
In-Reply-To: <aPuWev3D9M4iGCUt@nathan>
From: David Rowley <dgrowleyml@gmail.com>
Date: Sun, 26 Oct 2025 14:25:48 +1300
Message-ID: 
 <CAApHDvoM5MEHHBc0TNdrzkpq39WdEHSZhdWrtnx9zOWNXTSFGw@mail.gmail.com>
Subject: Re: another autovacuum scheduling thread
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Sami Imseih <samimseih@gmail.com>, Robert Haas <robertmhaas@gmail.com>,
	Jeremy Schneider <schneider@ardentperf.com>, pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvoM5MEHHBc0TNdrzkpq39WdEHSZhdWrtnx9zOWNXTSFGw%40mail.gmail.com>
Precedence: bulk

On Sat, 25 Oct 2025 at 04:08, Nathan Bossart <nathandbossart@gmail.com> wrote:
> Here is an updated patch based on the latest discussion.

Thanks. I've just had a look at it. A few comments and questions.

1) The subtraction here looks back to front:

+ xid_age = TransactionIdIsNormal(relfrozenxid) ? relfrozenxid - recentXid : 0;
+ mxid_age = MultiXactIdIsValid(relminmxid) ? relminmxid - recentMulti : 0;

2) Would it be better to move all the code that sets the xid_score and
mxid_score to under an "if (force_vacuum)"? Those two variables could
be declared in there too.

3) Could the following be refactored a bit so we only check the "relid
!= StatisticRelationId" condition once?

+ if (relid != StatisticRelationId &&
+ classForm->relkind != RELKIND_TOASTVALUE)

Something like:

/* ANALYZE refuses to work with pg_statistic and we don't analyze
toast tables */
if (anltuples > anlthresh && relid != StatisticRelationId &&
    classForm->relkind != RELKIND_TOASTVALUE)
{
    *doanalyze = true;
    // calc analyze score and Max with *score
}
else
  *doanalyze = false;

then delete:

/* ANALYZE refuses to work with pg_statistic */
if (relid == StatisticRelationId)
    *doanalyze = false;

4) Should these be TransactionIds?

+ uint32 xid_age;
+ uint32 mxid_age;

5) Instead of:

+ double score = 0.0;

Is it better to zero the score inside relation_needs_vacanalyze() so
it works the same as the other output parameters?

David