Re: POC: track vacuum/analyze cumulative time per relation

On Mon, Jan 20, 2025 at 11:04:40AM -0600, Sami Imseih wrote:
>> +       PgStat_Counter total_vacuum_time;
>> +       PgStat_Counter total_autovacuum_time;
>> +       PgStat_Counter total_analyze_time;
>> +       PgStat_Counter total_autoanalyze_time;
>>  } PgStat_StatTabEntry;
>> These are time values in milliseconds.  It would be good to document
>> that as a comment with these fields, like PgStat_CheckpointerStats and
>> others.  Perhaps they should be grouped with their related counters
>> within PgStat_StatTabEntry?  Or not.
> 
> done, including grouping all the vacuum counters together.

I was referring to the order of the fields in the structure itself,
but that's no big deal one way or the other.

> See the attached v6.

+    PgStat_Counter total_vacuum_time; /* times in milliseconds */
+    PgStat_Counter total_autovacuum_time; /* times in milliseconds */
+    PgStat_Counter total_analyze_time; /* times in milliseconds */
+    PgStat_Counter total_autoanalyze_time; /* times in milliseconds */

This should be one comment for the whole block, or this should use the
singular for each comment.

     instrument = (verbose || (AmAutoVacuumWorkerProcess() &&
                               params->log_min_duration >= 0));
+
     if (instrument)

Some noise.

     if (instrument)
     {
-        TimestampTz endtime = GetCurrentTimestamp();

On HEAD in the ANALYZE path, the endtime is calculated after
index_vacuum_cleanup().  Your patch calculates it before
index_vacuum_cleanup(), which would result in an incorrect calculation
if the index cleanup takes a long time with less logs generated, no?
Sorry for not noticing that earlier on the thread..  Perhaps it would
be just better to pass the start time to pgstat_report_vacuum() and
pgstat_report_analyze() then let their internals do the elapsed time
calculations.  Consistency of the arguments for both functions is
something worth having, IMO, even if it means a bit more
GetCurrentTimestamp() in this case.
--
Michael