Обсуждение: Enhance statistics reset functions to return reset timestamp

Поиск
Список
Период
Сортировка

Enhance statistics reset functions to return reset timestamp

От
Shinya Kato
Дата:
Hi hackers,

I would like to propose a series of patches that enhance the behavior
of various statistics reset functions by making them return the reset
timestamp. This change improves usability and aligns with the behavior
introduced in commit dc9f8a798[1], where pg_stat_statements_reset()
was updated to return the reset time.

The following functions have been modified to return a TIMESTAMP WITH
TIME ZONE value indicating when the statistics were reset:
- pg_stat_reset()
- pg_stat_reset_shared()
- pg_stat_reset_single_table_counters()
- pg_stat_reset_backend_stats()
- pg_stat_reset_single_function_counters()
- pg_stat_reset_slru()
- pg_stat_reset_replication_slot()
- pg_stat_reset_subscription_stats()
- pg_stat_clear_snapshot()

For pg_stat_reset_backend_stats() and
pg_stat_reset_replication_slot(), the functions return the reset
timestamp when a valid input is provided. If an invalid input is given
(e.g., an invalid backend PID or replication slot name), the functions
return NULL.
This allows users to easily determine whether the reset operation was
successful based on the return value.

Thoughts?

[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=dc9f8a798307244d791b71f666f50de319799e7c
--
Best regards,
Shinya Kato
NTT OSS Center

Вложения

Re: Enhance statistics reset functions to return reset timestamp

От
Michael Paquier
Дата:
On Fri, Aug 08, 2025 at 01:18:39PM +0900, Shinya Kato wrote:
> The following functions have been modified to return a TIMESTAMP WITH
> TIME ZONE value indicating when the statistics were reset:
> - pg_stat_reset()
> - pg_stat_reset_shared()
> - pg_stat_reset_single_table_counters()
> - pg_stat_reset_backend_stats()
> - pg_stat_reset_single_function_counters()
> - pg_stat_reset_slru()
> - pg_stat_reset_replication_slot()
> - pg_stat_reset_subscription_stats()
> - pg_stat_clear_snapshot()
>
> For pg_stat_reset_backend_stats() and
> pg_stat_reset_replication_slot(), the functions return the reset
> timestamp when a valid input is provided. If an invalid input is given
> (e.g., an invalid backend PID or replication slot name), the functions
> return NULL.
> This allows users to easily determine whether the reset operation was
> successful based on the return value.

I am not sure that this is a good idea overall, but I'm OK if I finish
outvoted.

At the exception of pg_stat_reset_backend_stats(), all the functions
you are listing above are written so as they never fail, so claiming
that returning a timestamp value for this reason sounds a bit strange
to me.  And we already know this information based on what the stats
tables already hold for the reset timestamps set when their stats
kinds callbacks are called.

I can get behind a change for pg_stat_reset_backend_stats() to make it
return a status, though, as the information depends on some procnum
lookups which is triggered depending on what the user provides in
input.
--
Michael

Вложения

Re: Enhance statistics reset functions to return reset timestamp

От
Shinya Kato
Дата:
On Fri, Aug 8, 2025 at 5:06 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Fri, Aug 08, 2025 at 01:18:39PM +0900, Shinya Kato wrote:
> > The following functions have been modified to return a TIMESTAMP WITH
> > TIME ZONE value indicating when the statistics were reset:
> > - pg_stat_reset()
> > - pg_stat_reset_shared()
> > - pg_stat_reset_single_table_counters()
> > - pg_stat_reset_backend_stats()
> > - pg_stat_reset_single_function_counters()
> > - pg_stat_reset_slru()
> > - pg_stat_reset_replication_slot()
> > - pg_stat_reset_subscription_stats()
> > - pg_stat_clear_snapshot()
> >
> > For pg_stat_reset_backend_stats() and
> > pg_stat_reset_replication_slot(), the functions return the reset
> > timestamp when a valid input is provided. If an invalid input is given
> > (e.g., an invalid backend PID or replication slot name), the functions
> > return NULL.
> > This allows users to easily determine whether the reset operation was
> > successful based on the return value.
>
> I am not sure that this is a good idea overall, but I'm OK if I finish
> outvoted.

Thanks for the review!

> At the exception of pg_stat_reset_backend_stats(), all the functions
> you are listing above are written so as they never fail, so claiming
> that returning a timestamp value for this reason sounds a bit strange
> to me.  And we already know this information based on what the stats
> tables already hold for the reset timestamps set when their stats
> kinds callbacks are called.

You are correct that these functions don't fail, so the lack of a
return value is not an issue. However, it would be more convenient,
for instance, when running these functions before and after a test
script, to get an audit trail of the reset time just by checking the
return value, without having to check the pg_stats_* views.

> I can get behind a change for pg_stat_reset_backend_stats() to make it
> return a status, though, as the information depends on some procnum
> lookups which is triggered depending on what the user provides in
> input.

The same applies to pg_stat_reset_replication_slot(). While specifying
a physical replication slot does not raise an error, the operation has
no effect. I think it would be better if this outcome could be
confirmed via the return value.

--
Best regards,
Shinya Kato
NTT OSS Center



Re: Enhance statistics reset functions to return reset timestamp

От
Andres Freund
Дата:
Hi,

On 2025-08-08 13:18:39 +0900, Shinya Kato wrote:
> I would like to propose a series of patches that enhance the behavior
> of various statistics reset functions by making them return the reset
> timestamp. This change improves usability and aligns with the behavior
> introduced in commit dc9f8a798[1], where pg_stat_statements_reset()
> was updated to return the reset time.
> 
> The following functions have been modified to return a TIMESTAMP WITH
> TIME ZONE value indicating when the statistics were reset:
> - pg_stat_reset()
> - pg_stat_reset_shared()
> - pg_stat_reset_single_table_counters()
> - pg_stat_reset_backend_stats()
> - pg_stat_reset_single_function_counters()
> - pg_stat_reset_slru()
> - pg_stat_reset_replication_slot()
> - pg_stat_reset_subscription_stats()
> - pg_stat_clear_snapshot()

-1 - I think it was a mistake to introduce support for granular resets, we
shouldn't bury ourselves deeper.  If anything we should rip out everything
other than 1) a global reset b) a per-database reset.

Leaving that aside, I just don't see a convincing use case for returning the
timestamp here.

Greetings,

Andres Freund



Re: Enhance statistics reset functions to return reset timestamp

От
Shinya Kato
Дата:
On Sat, Aug 9, 2025 at 7:37 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2025-08-08 13:18:39 +0900, Shinya Kato wrote:
> > I would like to propose a series of patches that enhance the behavior
> > of various statistics reset functions by making them return the reset
> > timestamp. This change improves usability and aligns with the behavior
> > introduced in commit dc9f8a798[1], where pg_stat_statements_reset()
> > was updated to return the reset time.
> >
> > The following functions have been modified to return a TIMESTAMP WITH
> > TIME ZONE value indicating when the statistics were reset:
> > - pg_stat_reset()
> > - pg_stat_reset_shared()
> > - pg_stat_reset_single_table_counters()
> > - pg_stat_reset_backend_stats()
> > - pg_stat_reset_single_function_counters()
> > - pg_stat_reset_slru()
> > - pg_stat_reset_replication_slot()
> > - pg_stat_reset_subscription_stats()
> > - pg_stat_clear_snapshot()
>
> -1 - I think it was a mistake to introduce support for granular resets, we
> shouldn't bury ourselves deeper.  If anything we should rip out everything
> other than 1) a global reset b) a per-database reset.
>
> Leaving that aside, I just don't see a convincing use case for returning the
> timestamp here.

As I mentioned earlier, the use case is to obtain an audit trail of
the statistics reset time from the function’s return value.

Also, how should we think about consistency with
pg_stat_statements_reset()? The reset timestamp for pg_stat_statements
can be obtained from the pg_stat_statements_info view, which returns a
TIMESTAMPTZ. Of course, pg_stat_statements is a contrib module, so we
don’t necessarily have to match its behavior. However, when running a
script that resets various statistics before tests, if only
pg_stat_statements_reset() returns a TIMESTAMPTZ, it might look as
though the other statistics reset functions failed.

What these patches do is simply return the TIMESTAMPTZ that is already
computed for the pg_stat_* views, which seems to me a reasonable
change.

--
Best regards,
Shinya Kato
NTT OSS Center



Re: Enhance statistics reset functions to return reset timestamp

От
Shinya Kato
Дата:
Hi,

On Wed, Aug 13, 2025 at 4:06 PM Shinya Kato <shinya11.kato@gmail.com> wrote:
>
> On Sat, Aug 9, 2025 at 7:37 AM Andres Freund <andres@anarazel.de> wrote:
> >
> > Hi,
> >
> > On 2025-08-08 13:18:39 +0900, Shinya Kato wrote:
> > > I would like to propose a series of patches that enhance the behavior
> > > of various statistics reset functions by making them return the reset
> > > timestamp. This change improves usability and aligns with the behavior
> > > introduced in commit dc9f8a798[1], where pg_stat_statements_reset()
> > > was updated to return the reset time.
> > >
> > > The following functions have been modified to return a TIMESTAMP WITH
> > > TIME ZONE value indicating when the statistics were reset:
> > > - pg_stat_reset()
> > > - pg_stat_reset_shared()
> > > - pg_stat_reset_single_table_counters()
> > > - pg_stat_reset_backend_stats()
> > > - pg_stat_reset_single_function_counters()
> > > - pg_stat_reset_slru()
> > > - pg_stat_reset_replication_slot()
> > > - pg_stat_reset_subscription_stats()
> > > - pg_stat_clear_snapshot()
> >
> > -1 - I think it was a mistake to introduce support for granular resets, we
> > shouldn't bury ourselves deeper.  If anything we should rip out everything
> > other than 1) a global reset b) a per-database reset.
> >
> > Leaving that aside, I just don't see a convincing use case for returning the
> > timestamp here.
>
> As I mentioned earlier, the use case is to obtain an audit trail of
> the statistics reset time from the function’s return value.
>
> Also, how should we think about consistency with
> pg_stat_statements_reset()? The reset timestamp for pg_stat_statements
> can be obtained from the pg_stat_statements_info view, which returns a
> TIMESTAMPTZ. Of course, pg_stat_statements is a contrib module, so we
> don’t necessarily have to match its behavior. However, when running a
> script that resets various statistics before tests, if only
> pg_stat_statements_reset() returns a TIMESTAMPTZ, it might look as
> though the other statistics reset functions failed.
>
> What these patches do is simply return the TIMESTAMPTZ that is already
> computed for the pg_stat_* views, which seems to me a reasonable
> change.

Rebased the patches.

--
Best regards,
Shinya Kato
NTT OSS Center

Вложения