Обсуждение: Enhance statistics reset functions to return reset timestamp
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
Вложения
- v1-0001-Make-pg_stat_reset-return-the-reset-time.patch
- v1-0002-Make-pg_stat_reset_shared-return-the-reset-time.patch
- v1-0003-Make-pg_stat_reset_single_table_counters-return-t.patch
- v1-0004-Make-pg_stat_reset_backend_stats-return-the-reset.patch
- v1-0005-Make-pg_stat_reset_single_function_counters-retur.patch
- v1-0006-Make-pg_stat_reset_slru-return-the-reset-time.patch
- v1-0007-Make-pg_stat_reset_replication_slot-return-the-re.patch
- v1-0008-Make-pg_stat_reset_subscription_stats-return-the-.patch
- v1-0009-Make-pg_stat_clear_snapshot-return-the-reset-time.patch
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
Вложения
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
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
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
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
Вложения
- v2-0001-Make-pg_stat_reset-return-the-reset-time.patch
- v2-0002-Make-pg_stat_reset_shared-return-the-reset-time.patch
- v2-0003-Make-pg_stat_reset_single_table_counters-return-t.patch
- v2-0004-Make-pg_stat_reset_backend_stats-return-the-reset.patch
- v2-0005-Make-pg_stat_reset_single_function_counters-retur.patch
- v2-0006-Make-pg_stat_reset_slru-return-the-reset-time.patch
- v2-0007-Make-pg_stat_reset_replication_slot-return-the-re.patch
- v2-0008-Make-pg_stat_reset_subscription_stats-return-the-.patch
- v2-0009-Make-pg_stat_clear_snapshot-return-the-reset-time.patch