public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Sami Imseih <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: POC: track vacuum/analyze cumulative time per relation
Date: Mon, 20 Jan 2025 12:25:17 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA5RZ0vKWKULGZ_V8DOOtFHXtsOLMVLCpkJ+guniB66T-o_eZA@mail.gmail.com>
References: <CAA5RZ0uVOGBYmPEeGF2d1B_67tgNjKx_bKDuL+oUftuoz+=Y1g@mail.gmail.com>
<[email protected]>
<CAA5RZ0tP6my7Mbr_syP7hRAiFANC_bG8SyoQF=-vfV9VawpV=g@mail.gmail.com>
<[email protected]>
<CAA5RZ0v9060p7hjtLPY9i0Me6SoaRKYwxATqt_DnZr8dGSnUhQ@mail.gmail.com>
<Z4ez/[email protected]>
<CAA5RZ0s8=wd-X4GgMcFQO1WbwPtw4HjTtMcDahsnegZ8p6tYPQ@mail.gmail.com>
<[email protected]>
<CAA5RZ0vKWKULGZ_V8DOOtFHXtsOLMVLCpkJ+guniB66T-o_eZA@mail.gmail.com>
On Fri, Jan 17, 2025 at 09:42:13AM -0600, Sami Imseih wrote:
> Indeed. Corrected in v5
I can get behind the idea of the patch.
+ 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.
The patch makes sense here in terms of passing down to
pgstat_report_vacuum() the elapsed time and the end time to avoid an
extra GetCurrentTimestamp() when calculating if the logging should
happen. However in analyze.c..
+++ b/src/backend/commands/analyze.c
[...]
+ TimestampTz endtime = 0;
[...]
- TimestampTz endtime = GetCurrentTimestamp();
-
if (verbose || params->log_min_duration == 0 ||
TimestampDifferenceExceeds(starttime, endtime,
This is incorrect, endtime is never set. I would give priority to the
symmetry of the code here, using for the analyze reporting the same
arguments as the vacuum bits, so as we don't call GetCurrentTimestamp
in the proposed changes for pgstat_report_analyze().
+ if (AmAutoVacuumWorkerProcess())
+ tabentry->total_autovacuum_time += elapsedtime;
+ else
+ tabentry->total_vacuum_time += elapsedtime;
[...]
+ if (AmAutoVacuumWorkerProcess())
+ tabentry->total_autoanalyze_time += elapsedtime;
+ else
+ tabentry->total_analyze_time += elapsedtime;
These could be grouped with their previous blocks.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (19+ 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]
Subject: Re: POC: track vacuum/analyze cumulative time per relation
In-Reply-To: <[email protected]>
* 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