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

On Fri, Jan 24, 2025 at 02:19:59PM -0600, Sami Imseih wrote:
> So currently, the report of the last_(autoanalyze|analyze)_time
> is set before the index_vacuum_cleanup, but for logging purposes
> the elapsed time is calculated afterwards. Most users will not notice
> this, but I think that is wrong as well.
>
> I think we should calculate endtime and elapsedtime and call
> pgstat_report_analyze after the index_vacuum_cleanup; and
> before vac_close_indexes. This is more accurate and will
> avoid incurring the extra GetCurrentTimestamp() call.
>
> What do you think?

I think that this is hiding a behavior change while adding new
counters, and both changes are independent. Another thing that is
slightly incorrect to do if we take the argument of only adding the
counters is moving around the call of pgstat_progress_end_command(),
because it's not really wrong as-is, either. I'd suggest to make all
that a separate discussion.

The addition of the extra GetCurrentTimestamp() in the report path
does not stress me much, FWIW, and keeping the tracking of the end
time within the report routines for [auto]analyze and [auto]vacuum has
the merit that the caller does not need to worry about more
TimestampDifferenceMilliseconds() calculations.

I have put my hands on this patch, and at the end I think that we
should just do the attached, which is simpler and addresses your
use-case. Note also that the end time is acquired while the entries
are not locked in the report routines, and some tweaks in the docs and
comments.
--
Michael