Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-16 Thread torikoshia
On 2023-11-16 16:48, Michael Paquier wrote: On Wed, Nov 15, 2023 at 04:25:14PM +0900, Michael Paquier wrote: Other than that, it looks OK. Tweaked the queries of this one slightly, and applied. So I think that we are now good for this thread. Thanks, all! Thanks for the modification and

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-15 Thread Michael Paquier
On Wed, Nov 15, 2023 at 04:25:14PM +0900, Michael Paquier wrote: > Other than that, it looks OK. Tweaked the queries of this one slightly, and applied. So I think that we are now good for this thread. Thanks, all! -- Michael signature.asc Description: PGP signature

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-14 Thread Michael Paquier
On Wed, Nov 15, 2023 at 11:58:38AM +0900, torikoshia wrote: > On 2023-11-15 09:47, Michael Paquier wrote: >> You have forgotten to update the errhint at the end of >> pg_stat_reset_shared(), where "slru" needs to be listed :) > > Oops, attached v2 patch. +SELECT stats_reset >

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-14 Thread torikoshia
On 2023-11-15 09:47, Michael Paquier wrote: On Tue, Nov 14, 2023 at 10:02:32PM +0900, torikoshia wrote: Attached patch. You have forgotten to update the errhint at the end of pg_stat_reset_shared(), where "slru" needs to be listed :) Oops, attached v2 patch. BTW currently the

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-14 Thread Michael Paquier
On Tue, Nov 14, 2023 at 10:02:32PM +0900, torikoshia wrote: > Attached patch. You have forgotten to update the errhint at the end of pg_stat_reset_shared(), where "slru" needs to be listed :) > BTW currently the documentation explains all the arguments of > pg_stat_reset_shared() in one line and

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-14 Thread torikoshia
On 2023-11-13 13:15, torikoshia wrote: On 2023-11-12 16:46, Michael Paquier wrote: On Fri, Nov 10, 2023 at 01:15:50PM +0900, Michael Paquier wrote: The comments added could be better grammatically, but basically LGTM. I'll take care of that if there are no objections. The documentation also

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-13 Thread Michael Paquier
On Mon, Nov 13, 2023 at 07:31:41PM +0900, Michael Paquier wrote: > On Mon, Nov 13, 2023 at 02:07:21PM +0530, Bharath Rupireddy wrote: >> Modified the docs for pg_stat_reset_slru to match with that of >> pg_stat_reset_shared. PSA v2 patch. > > That feels consistent. Thanks. And applied this one.

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-13 Thread Michael Paquier
On Mon, Nov 13, 2023 at 02:07:21PM +0530, Bharath Rupireddy wrote: > Modified the docs for pg_stat_reset_slru to match with that of > pg_stat_reset_shared. PSA v2 patch. That feels consistent. Thanks. > I noticed that the commit 23c8c0c8 missed to add proargnames => > '{target}' in .dat file

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-13 Thread Bharath Rupireddy
On Mon, Nov 13, 2023 at 9:45 AM torikoshia wrote: > > On 2023-11-12 16:46, Michael Paquier wrote: > > On Fri, Nov 10, 2023 at 01:15:50PM +0900, Michael Paquier wrote: > >> The comments added could be better grammatically, but basically LGTM. > >> I'll take care of that if there are no objections.

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-12 Thread Michael Paquier
On Mon, Nov 13, 2023 at 01:15:14PM +0900, torikoshia wrote: > I assume you have already taken this into account, but I think we should add > the same documentation to the below patch for pg_stat_reset_slru(): > >

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-12 Thread torikoshia
On 2023-11-12 16:46, Michael Paquier wrote: On Fri, Nov 10, 2023 at 01:15:50PM +0900, Michael Paquier wrote: The comments added could be better grammatically, but basically LGTM. I'll take care of that if there are no objections. The documentation also needed a few tweaks (for DEFAULT and the

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-11 Thread Michael Paquier
On Fri, Nov 10, 2023 at 08:32:34PM +0900, torikoshia wrote: > On 2023-11-10 13:18, Andres Freund wrote: >> 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

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-11 Thread Michael Paquier
On Fri, Nov 10, 2023 at 01:15:50PM +0900, Michael Paquier wrote: > The comments added could be better grammatically, but basically LGTM. > I'll take care of that if there are no objections. The documentation also needed a few tweaks (for DEFAULT and the argument name), so I have fixed the whole

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-10 Thread torikoshia
On 2023-11-10 13:18, Andres Freund wrote: Hi, On November 8, 2023 11:28:08 PM PST, Michael Paquier 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

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-09 Thread Andres Freund
Hi, On November 8, 2023 11:28:08 PM PST, Michael Paquier 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()

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-09 Thread Michael Paquier
On Fri, Nov 10, 2023 at 12:33:50PM +0900, torikoshia wrote: > On 2023-11-09 16:28, Michael Paquier wrote: >> Not sure how to feel about that, TBH, but I would not include SLRUs >> here if we have already a separate function. > > IMHO I agree with you. The comments added could be better

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-09 Thread torikoshia
On 2023-11-09 16:28, Michael Paquier wrote: Thanks for your review. Attached v2 patch. 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

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread Michael Paquier
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

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread torikoshia
On Thu, Nov 09, 2023 at 10:10:39AM +0900, torikoshia wrote: I'll attach the patch. Attached. On Mon, Nov 6, 2023 at 5:30 PM Bharath Rupireddy 3. I think the new reset all stats function must also consider resetting all SLRU stats, no? /* stats for fixed-numbered objects */

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread Andres Freund
Hi, On 2023-11-09 10:25:18 +0900, Michael Paquier wrote: > On Thu, Nov 09, 2023 at 10:10:39AM +0900, torikoshia wrote: > > I am a little concerned about that the reset time is not the same and that > > GetCurrentTimestamp() is called multiple times, but I think it would be > > acceptable because

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread Michael Paquier
On Thu, Nov 09, 2023 at 10:10:39AM +0900, torikoshia wrote: > I am a little concerned about that the reset time is not the same and that > GetCurrentTimestamp() is called multiple times, but I think it would be > acceptable because the function is probably not used that often and the > reset time

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread torikoshia
On 2023-11-09 08:58, Michael Paquier wrote: On Wed, Nov 08, 2023 at 02:15:24PM +0530, Bharath Rupireddy wrote: On Wed, Nov 8, 2023 at 9:43 AM Andres Freund wrote: It's not like oids are a precious resource. It's a more confusing API to have to have to specify a NULL as an argument than not

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread Michael Paquier
On Wed, Nov 08, 2023 at 02:15:24PM +0530, Bharath Rupireddy wrote: > On Wed, Nov 8, 2023 at 9:43 AM Andres Freund wrote: >> It's not like oids are a precious resource. It's a more confusing API to have >> to have to specify a NULL as an argument than not having to do so. If we >> really want to

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread Matthias van de Meent
On Wed, 8 Nov 2023 at 05:13, Andres Freund wrote: > > Hi, > > On 2023-11-06 14:00:13 +0530, Bharath Rupireddy wrote: > > Well, that's a total of ~17 LWLocks this new function takes to make > > the stats reset atomic. I'm not sure if this atomicity is worth the > > effort which can easily be

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-08 Thread Bharath Rupireddy
On Wed, Nov 8, 2023 at 9:43 AM Andres Freund wrote: > > > 2. > > +{ oid => '8000', > > + descr => 'statistics: reset collected statistics shared across the > > cluster', > > + proname => 'pg_stat_reset_shared', provolatile => 'v', prorettype => > > 'void', > > + proargtypes => '', prosrc =>

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-07 Thread Andres Freund
Hi, On 2023-11-08 10:08:42 +0900, Kyotaro Horiguchi wrote: > At Mon, 6 Nov 2023 14:00:13 +0530, Bharath Rupireddy > wrote in > > On Mon, Nov 6, 2023 at 11:39 AM torikoshia > > wrote: > > > Since each stats, except wal_prefetch was reset acquiring LWLock, > > > attached PoC patch makes the

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-07 Thread Andres Freund
Hi, On 2023-11-06 14:00:13 +0530, Bharath Rupireddy wrote: > On Mon, Nov 6, 2023 at 11:39 AM torikoshia wrote: > > > > Thanks all for the comments! > > > > On Fri, Nov 3, 2023 at 5:17 AM Matthias van de Meent > > wrote: > > > Knowing that your metrics have a shared starting point can be quite >

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-07 Thread Michael Paquier
On Wed, Nov 08, 2023 at 10:08:42AM +0900, Kyotaro Horiguchi wrote: > At Mon, 6 Nov 2023 14:00:13 +0530, Bharath Rupireddy > wrote in > I must say, I have reservations about this approach. The main concern > is the duplication of reset code, which has been efficiently > encapsulated for

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-07 Thread Kyotaro Horiguchi
At Mon, 6 Nov 2023 14:00:13 +0530, Bharath Rupireddy wrote in > On Mon, Nov 6, 2023 at 11:39 AM torikoshia wrote: > > Since each stats, except wal_prefetch was reset acquiring LWLock, > > attached PoC patch makes the call atomic by using these LWlocks. > > > > If this is the right direction,

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-06 Thread Bharath Rupireddy
On Mon, Nov 6, 2023 at 11:39 AM torikoshia wrote: > > Thanks all for the comments! > > On Fri, Nov 3, 2023 at 5:17 AM Matthias van de Meent > wrote: > > Knowing that your metrics have a shared starting point can be quite > > valuable, as it allows you to do some math that would otherwise be > >

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-05 Thread Bharath Rupireddy
On Sat, Nov 4, 2023 at 7:19 AM Andres Freund wrote: > > On 2023-11-03 00:55:00 +0530, Bharath Rupireddy wrote: > > On Wed, Nov 1, 2023 at 4:24 AM Michael Paquier wrote: > > > > > > On Tue, Oct 31, 2023 at 04:26:18PM +0900, torikoshia wrote: > > > > Yes, calling pg_stat_reset_shared() for all

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-05 Thread torikoshia
Thanks all for the comments! On Fri, Nov 3, 2023 at 5:17 AM Matthias van de Meent wrote: Knowing that your metrics have a shared starting point can be quite valuable, as it allows you to do some math that would otherwise be much less accurate when working with stats over a short amount of

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-03 Thread Andres Freund
Hi, On 2023-11-03 00:55:00 +0530, Bharath Rupireddy wrote: > On Wed, Nov 1, 2023 at 4:24 AM Michael Paquier wrote: > > > > On Tue, Oct 31, 2023 at 04:26:18PM +0900, torikoshia wrote: > > > Yes, calling pg_stat_reset_shared() for all stats types can do what I > > > wanted > > > to do. > > > But

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-02 Thread Matthias van de Meent
On Thu, 2 Nov 2023 at 20:26, Bharath Rupireddy wrote: > > On Wed, Nov 1, 2023 at 4:24 AM Michael Paquier wrote: > > > > On Tue, Oct 31, 2023 at 04:26:18PM +0900, torikoshia wrote: > > > Yes, calling pg_stat_reset_shared() for all stats types can do what I > > > wanted > > > to do. > > > But

Re: Add new option 'all' to pg_stat_reset_shared()

2023-11-02 Thread Bharath Rupireddy
On Wed, Nov 1, 2023 at 4:24 AM Michael Paquier wrote: > > On Tue, Oct 31, 2023 at 04:26:18PM +0900, torikoshia wrote: > > Yes, calling pg_stat_reset_shared() for all stats types can do what I wanted > > to do. > > But calling it with 6 different parameters seems tiresome and I thought it > >

Re: Add new option 'all' to pg_stat_reset_shared()

2023-10-31 Thread Michael Paquier
On Tue, Oct 31, 2023 at 04:26:18PM +0900, torikoshia wrote: > Yes, calling pg_stat_reset_shared() for all stats types can do what I wanted > to do. > But calling it with 6 different parameters seems tiresome and I thought it > would be convenient to have a parameter to delete all cluster-wide >

Re: Add new option 'all' to pg_stat_reset_shared()

2023-10-31 Thread torikoshia
On Mon, Oct 30, 2023 at 5:46 PM Bharath Rupireddy wrote: Thanks for the comments! Isn't calling pg_stat_reset_shared() for all stats types helping you out? Is there any problem with it? Can you be more specific about the use-case? Yes, calling pg_stat_reset_shared() for all stats types can

Re: Add new option 'all' to pg_stat_reset_shared()

2023-10-30 Thread Kyotaro Horiguchi
At Mon, 30 Oct 2023 14:15:53 +0530, Bharath Rupireddy wrote in > > > I imagine there are cases where people want to initialize all of them > > > at the same time in addition to initializing one at a time. > > > > FWIW, I fairly often wanted it, but forgot about that:p > > Isn't calling

Re: Add new option 'all' to pg_stat_reset_shared()

2023-10-30 Thread Bharath Rupireddy
On Mon, Oct 30, 2023 at 1:47 PM Kyotaro Horiguchi wrote: > > At Mon, 30 Oct 2023 16:35:19 +0900, torikoshia > wrote in > > Hi, > > > > After 96f052613f3, we have below 6 types of parameter for > > pg_stat_reset_shared(). > > > > "archiver", "bgwriter", "checkpointer", "io",

Re: Add new option 'all' to pg_stat_reset_shared()

2023-10-30 Thread Kyotaro Horiguchi
At Mon, 30 Oct 2023 16:35:19 +0900, torikoshia wrote in > Hi, > > After 96f052613f3, we have below 6 types of parameter for > pg_stat_reset_shared(). > > "archiver", "bgwriter", "checkpointer", "io", "recovery_prefetch", > "wal" > > How about adding a new option 'all' to delete all

Add new option 'all' to pg_stat_reset_shared()

2023-10-30 Thread torikoshia
Hi, After 96f052613f3, we have below 6 types of parameter for pg_stat_reset_shared(). "archiver", "bgwriter", "checkpointer", "io", "recovery_prefetch", "wal" How about adding a new option 'all' to delete all targets above? I imagine there are cases where people want to initialize all