Re: pg_stat_reset_single_*_counters vs pg_stat_database.stats_reset

From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Magnus Hagander <magnus(at)hagander(dot)net>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Tomas Vondra <tv(at)fuzzy(dot)cz>
Subject: Re: pg_stat_reset_single_*_counters vs pg_stat_database.stats_reset
Date: 2022-03-24 01:47:58
Message-ID: CAKFQuwZDzxJ=SAHx0jF08ibZj_S6CG41Rq-RK_Xtnom2u6O0KA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 23, 2022 at 5:55 PM Andres Freund <andres(at)anarazel(dot)de> wrote:

>
> Starting with the below commit, pg_stat_reset_single_function_counters,
> pg_stat_reset_single_table_counters don't just reset the stats for the
> individual function, but also set pg_stat_database.stats_reset.
>
> commit 4c468b37a281941afd3bf61c782b20def8c17047
> Author: Magnus Hagander <magnus(at)hagander(dot)net>
> Date: 2011-02-10 15:09:35 +0100
>
> Track last time for statistics reset on databases and bgwriter
>
> Tracks one counter for each database, which is reset whenever
> the statistics for any individual object inside the database is
> reset, and one counter for the background writer.
>
> Tomas Vondra, reviewed by Greg Smith
> /*
> [...]
> Maybe I just don't understand what these reset functions are intended for?
> Their introduction [3] didn't explain much either. To me the behaviour of
> resetting pg_stat_database.stats_reset but nothing else in pg_stat_database
> makes them kind of dangerous.
>

*tl/dr;*
There seems to be three scopes here:

Cluster Stats - (add last_reset fields for consistency)
Database and Shared Object Stats (add last_reset to handle recording the
resetting of just the records on this table, leaving stats_reset to be a
flag meaning self-or-children)
Object Stats (add last_reset, don't need stats_reset since there are no
children)

If we are OK with just changing pg_stat_database.stats_reset meanings to be
less informative than what it is today (I strongly dislike such a silent
behavioral change) we could simply standardize on the intended meaning of
stats_reset on all three scopes, adding stats_reset as needed to track
object-level resetting.

*Additional Exposition*

The description for the column declares that the field is reset when the
statistics on the pg_stat_database are reset. That is also the expected
behavior, knowing when any statistics in the whole database are reset is
indeed not useful.

"Time at which these statistics were last reset"

The "these" clearly refers to the statistics columns in pg_stat_database.

In fact, pg_stat_archiver.stats_reset already exists (as does
pg_bgwriter.stats_reset) with (I presume) this interpretation. This is a
problem because pg_stat_database.stats_reset does not have the same
meaning. So we have to either live with inconsistency or break something.

In the vein of living with inconsistency I would suggest changing the
documentation of "pg_stat.database.stats_reset" to match the present
behavior. Then add a new column (last_reset ?) to represent the existing
description of "stats_reset".

I suppose we could document "stats_reset" as the itself-or-any-children
reset timestamp, it's just that the archive and pgwriter don't have
children in this sense while databases do. When the
pg_stat_database.last_reset field changes the pg_stat_database.stats_reset
would have to match anyway.

I don't have any issue with an indicator field saying "something regarding
stats has changed" at the database level. It is much easier to monitor
that and then inspect what may have changed rather than monitoring a
last_reset column on every single catalog that has statistics that can be
reset.

It also seems that each tracked object type needs to have its own
last_reset field (we could choose to name it stats_reset too, leaving
pg_stat_database.last_reset as the only anomaly) added as an implied
behavior needed for such individualized resetting. I would go with
*.last_reset though and leave the absence of pg_stat_archiver.last_reset as
the anomaly (or just add it redundantly for consistency).

I don't see removing existing functionality as a good course to getting a
consistent implementation; we should just push forward with figuring out
what is missing and fill in those gaps. At worst if that isn't something
we want to fix right now our new setup should at least leave the status quo
behaviors in place.

I haven't looked into what kind of explicit resetting options are available
but the above seems to cover tracking resetting regardless of how it is
implemented. I've only spot checked some of the tables to identify the
pattern.

David J.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2022-03-24 02:03:38 Re: [PoC] Reducing planning time when tables have many partitions
Previous Message Kyotaro Horiguchi 2022-03-24 01:37:37 Re: Support isEmptyStringInfo