On 2023-11-10 13:18, Andres Freund wrote:
Hi,
On November 8, 2023 11:28:08 PM PST, Michael Paquier
<[email protected]> wrote:
On Thu, Nov 09, 2023 at 01:50:34PM +0900, torikoshia wrote:
PGSTAT_KIND_SLRU cannot be reset by pg_stat_reset_shared(), so I feel
uncomfortable to delete it all together.
It might be better after pg_stat_reset_shared() has been modified to
take
'slru' as an argument, though.
Not sure how to feel about that, TBH, but I would not include SLRUs
here if we have already a separate function.
I thought it would be better to reset statistics even when null is
specified
so that users are not confused with the behavior of
pg_stat_reset_slru().
Attached patch added pg_stat_reset_shared() in system_functions.sql
mainly
for this reason.
I'm OK with doing what your patch does, aka do the work if the value
is NULL or if there's no argument given.
- Resets some cluster-wide statistics counters to zero,
depending on the
+ Resets cluster-wide statistics counters to zero, depending on
the
This does not need to change, aka SLRUs are for example still global
and not included here.
+ If the argument is NULL or not specified, all counters shown
in all
+ of these views are reset.
Missing a <literal> markup around NULL. I know, we're not consistent
about that, either, but if we are tweaking the area let's be right at
least. Perhaps "all the counters from the views listed above are
reset"?
I see no reason to not include slrus. We should never have added the
ability to reset them individually, particularly not without a use
case - I couldn't find one skimming some discussion. And what's the
point in not allowing to reset them via pg_stat_reset_shared()?
When including SLRUs, do you think it's better to add 'slrus' argument
which enables pg_stat_reset_shared() to reset all SLRUs?
As described above, since SLRUs cannot be reset by
pg_stat_reset_shared(), I feel a bit uncomfortable to delete it all
together.
--
Regards,
--
Atsushi Torikoshi
NTT DATA Group Corporation
