| From: | torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, bharath(dot)rupireddyforpostgres(at)gmail(dot)com, boekewurm+postgres(at)gmail(dot)com, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Add new option 'all' to pg_stat_reset_shared() |
| Date: | 2023-11-10 11:32:34 |
| Message-ID: | 6c01af77007905c73751f42eea03aef5@oss.nttdata.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2023-11-10 13:18, Andres Freund wrote:
> Hi,
>
> On November 8, 2023 11:28:08 PM PST, Michael Paquier
> <michael(at)paquier(dot)xyz> 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matthias van de Meent | 2023-11-10 12:27:58 | Re: Parallel aggregates in PG 16.1 |
| Previous Message | Dilip Kumar | 2023-11-10 11:27:14 | Re: trying again to get incremental backup |