On Sun, Oct 26, 2025 at 02:25:48PM +1300, David Rowley wrote:
> Thanks. I've just had a look at it. A few comments and questions.
Thanks.
> 1) The subtraction here looks back to front:
>
> + xid_age = TransactionIdIsNormal(relfrozenxid) ? relfrozenxid - recentXid : 0;
> + mxid_age = MultiXactIdIsValid(relminmxid) ? relminmxid - recentMulti : 0;
D'oh.
> 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.
Seems reasonable.
> 3) Could the following be refactored a bit so we only check the "relid
> != StatisticRelationId" condition once?
Yes. We can update the vacuum part to follow the same pattern, too.
> 4) Should these be TransactionIds?
>
> + uint32 xid_age;
> + uint32 mxid_age;
Probably.
> 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?
My only concern about this is that some compilers might complain about
potentially-uninitialized uses. But we can still zero it in the function
regardless.
--
nathan
