> I can get behind the idea of the patch. Thanks for the review!
> + 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. > This is incorrect, endtime is never set. indeed. fixed. > 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(). I agree. pgstat_report_analyze and pgstat_report_vacuum match will now take endtime and elapsedtime and we can remove the GetCurrentTimestamp inside 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; > Correct. fixed as well. See the attached v6. Thanks! Regards, Sami
v6-0001-Track-per-relation-cumulative-time-spent-in-vacuu.patch
Description: Binary data
