Re: [HACKERS] More stats about skipped vacuums

Thank you for reviewing this.

At Wed, 15 Nov 2017 16:13:01 +0900, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote in <CAB7nPqQm_WCKuUf5RD0CzeMuMO907ZPKP7mBh-3t2zSJ9jn+PA(at)mail(dot)gmail(dot)com>
> pg_stat_get_mod_since_analyze(C.oid) AS n_mod_since_analyze,
> + pg_stat_get_vacuum_necessity(C.oid) AS vacuum_required,
> pg_stat_get_last_vacuum_time(C.oid) as last_vacuum,
> pg_stat_get_last_autovacuum_time(C.oid) as last_autovacuum,
> pg_stat_get_last_analyze_time(C.oid) as last_analyze,
> pg_stat_get_last_autoanalyze_time(C.oid) as last_autoanalyze,
> pg_stat_get_vacuum_count(C.oid) AS vacuum_count,
> Please use spaces instead of tabs. Indentation is not consistent.

Done. Thank you for pointing. (whitespace-mode showed me some
similar inconsistencies at the other places in the file...)

> + case PGSTAT_VACUUM_CANCELED:
> + phase = tabentry->vacuum_last_phase;
> + /* number of elements of phasestr above */
> + if (phase >= 0 && phase <= 7)
> + result = psprintf("%s while %s",
> + status == PGSTAT_VACUUM_CANCELED ?
> + "canceled" : "error",
> + phasestr[phase]);
> Such complication is not necessary. The phase parameter is updated by
> individual calls of pgstat_progress_update_param(), so the information
> showed here overlaps with the existing information in the "phase"
> field.

The "phase" is pg_stat_progress_vacuum's? If "complexy" means
phasestr[phase], the "phase" cannot be overlap with
last_vacuum_status since pg_stat_progress_vacuum's entry has
already gone when someone looks into pg_stat_all_tables and see a
failed vacuum status. Could you give a bit specific comment?

> @@ -210,7 +361,6 @@ pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS)
> PG_RETURN_INT64(result);
> }
>
> -
> Datum
> pg_stat_get_blocks_hit(PG_FUNCTION_ARGS)
> Noise diff.

Removed.

> Thinking about trying to et something into core by the end of the
> commit fest, this patch presents multiple concepts at once which could
> be split into separate patches for simplicity:
> 1) Additional data fields to help in debugging completed vacuums.
> 2) Tracking of interrupted vacuum jobs in progress table.
> 3) Get state of vacuum job on error.
>
> However, progress reports are here to allow users to do decisions
> based on the activity of how things are working. This patch proposes
> to add multiple new fields:
> - oldest Xmin.
> - number of index scans.
> - number of pages truncated.
> - number of pages that should have been truncated, but are not truncated.
> Among all this information, as Sawada-san has already mentioned
> upthread, the more index scans the less dead tuples you can store at
> once, so autovacuum_work_mem ought to be increases. This is useful for
> tuning and should be documented properly if reported to give
> indications about vacuum behavior. The rest though, could indicate how
> aggressive autovacuum is able to remove tail blocks and do its work.
> But what really matters for users to decide if autovacuum should be
> more aggressive is tracking the number of dead tuples, something which
> is already evaluated.

Hmm. I tend to agree. Such numbers are better to be shown as
average of the last n vacuums or maximum. I decided to show
last_vacuum_index_scan only and I think that someone can record
it continuously to elsewhere if wants.

> Tracking the number of failed vacuum attempts is also something
> helpful to understand how much the job is able to complete. As there
> is already tracking vacuum jobs that have completed, it could be
> possible, instead of logging activity when a vacuum job has failed, to
> track the number of *begun* jobs on a relation. Then it is possible to
> guess how many have failed by taking the difference between those that
> completed properly. Having counters per failure types could also be a
> possibility.

Maybe pg_stat_all_tables is not the place to hold such many kinds
of vacuum specific information. pg_stat_vacuum_all_tables or
something like?

> For this commit fest, I would suggest a patch that simply adds
> tracking for the number of index scans done, with documentation to
> give recommendations about parameter tuning. i am switching the patch
> as "waiting on author".

Ok, the patch has been split into the following four parts. (Not
split by function, but by the kind of information to add.)
The first one is that.

0001. Adds pg_stat_all_tables.last_vacuum_index_scans. Documentation is added.

0002. Adds pg_stat_all_tables.vacuum_required. And primitive documentation.

0003. Adds pg_stat_all_tables.last_vacuum_status/autovacuum_fail_count
plus primitive documentation.

0004. truncation information stuff.

One concern on pg_stat_all_tables view is the number of
predefined functions it uses. Currently 20 functions and this
patch adds more seven. I feel it's better that at least the
functions this patch adds are merged into one function..

--
Kyotaro Horiguchi
NTT Open Source Software Center