Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-16 Thread Michael Paquier
On Thu, Jan 16, 2025 at 11:55:34AM +0300, Nazir Bilal Yavuz wrote: > I checked clang 4 as well on the link you sent and it also fixes the > warning there. Confirmed here, so done this way. -- Michael signature.asc Description: PGP signature

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-16 Thread Nazir Bilal Yavuz
Hi, On Thu, 16 Jan 2025 at 10:12, Bertrand Drouvot wrote: > > Hi, > > On Thu, Jan 16, 2025 at 12:47:17AM -0500, Tom Lane wrote: > > Michael Paquier writes: > > > Not completely sure about the number of parenthesis, but I hope that > > > this should be enough (extra set around io_op): > > > +#def

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Michael Paquier
On Thu, Jan 16, 2025 at 07:12:34AM +, Bertrand Drouvot wrote: > From what I can see, the above proposal does (at least) silent the warning > here (clang 5.0.1 and same as demoiselle): https://godbolt.org/z/cGosfzGne (we > can see the warning by using the current define and that the warning is g

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Bertrand Drouvot
Hi, On Thu, Jan 16, 2025 at 12:47:17AM -0500, Tom Lane wrote: > Michael Paquier writes: > > Not completely sure about the number of parenthesis, but I hope that > > this should be enough (extra set around io_op): > > +#define pgstat_is_ioop_tracked_in_bytes(io_op) \ > > + (((unsigned int) (io_o

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Tom Lane
Michael Paquier writes: > Not completely sure about the number of parenthesis, but I hope that > this should be enough (extra set around io_op): > +#define pgstat_is_ioop_tracked_in_bytes(io_op) \ > + (((unsigned int) (io_op)) < IOOP_NUM_TYPES && \ > + ((unsigned int) (io_op)) >= IOOP_EXT

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Michael Paquier
On Thu, Jan 16, 2025 at 12:18:38AM -0500, Tom Lane wrote: > However, the macro does provide a convenient place to hang the > warning comment about keeping it sync'd with the enum. > Personally I'd keep the macro but move it to pgstat.h, close > to the enum declaration, so that there's more than eps

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Tom Lane
Michael Paquier writes: > Just for an assert, I would just remove the macro rather than have an > inline function. Oh, I'd not noticed that there is only one caller. However, the macro does provide a convenient place to hang the warning comment about keeping it sync'd with the enum. Personally I

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Michael Paquier
On Wed, Jan 15, 2025 at 11:34:14PM -0500, Tom Lane wrote: > Michael Paquier writes: > > I cannot reproduce that, perhaps I'm just missing something with these > > switches. Do you think that a cast would cool things? Please see the > > attached for the idea. > > There are only three animals sho

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Tom Lane
Michael Paquier writes: > I cannot reproduce that, perhaps I'm just missing something with these > switches. Do you think that a cast would cool things? Please see the > attached for the idea. There are only three animals showing this warning (ayu, batfish, demoiselle) so it likely requires par

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Michael Paquier
On Wed, Jan 15, 2025 at 12:19:03PM -0500, Tom Lane wrote: > I don't see a reasonable way to alter that check to suppress this; > for instance, "(io_op) <= IOOP_WRITE" would probably still draw the > same warning. I think most likely we have to remove that check, ie > > #define pgstat_is_ioop_tra

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-15 Thread Tom Lane
Nazir Bilal Yavuz writes: > On Tue, 14 Jan 2025 at 06:18, Michael Paquier wrote: >> And I've somewhat managed to fat-finger the business with >> pgstat_count_io_op() with an incorrect rebase. Will remove in a >> minute.. > Thank you! Commit f92c854cf has caused some of the buildfarm members to

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-14 Thread Nazir Bilal Yavuz
Hi, On Tue, 14 Jan 2025 at 06:18, Michael Paquier wrote: > > On Fri, Jan 10, 2025 at 08:23:46AM +, Bertrand Drouvot wrote: > > On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote: > >> But I agree that having a macro has more benefits, > >> also there already is a check for the

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-13 Thread Michael Paquier
On Fri, Jan 10, 2025 at 08:23:46AM +, Bertrand Drouvot wrote: > On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote: >> But I agree that having a macro has more benefits, >> also there already is a check for the 'io_op < IOOP_NUM_TYPES' in the >> pgstat_count_io_op() function. >

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-10 Thread Bertrand Drouvot
Hi, On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote: > Hi, > > On Fri, 10 Jan 2025 at 04:47, Michael Paquier wrote: > > > > On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote: > > > We first use an Assert in is_ioop_tracked_in_bytes() and then we return > > > a va

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-09 Thread Nazir Bilal Yavuz
Hi, On Fri, 10 Jan 2025 at 04:47, Michael Paquier wrote: > > On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote: > > We first use an Assert in is_ioop_tracked_in_bytes() and then we return > > a value "just" to check another Assert. I wonder if it wouldn't make more > > sense > > t

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-09 Thread Michael Paquier
On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote: > We first use an Assert in is_ioop_tracked_in_bytes() and then we return > a value "just" to check another Assert. I wonder if it wouldn't make more > sense > to get rid of this function and use a macro instead, something like? >

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-09 Thread Bertrand Drouvot
Hi, On Thu, Jan 09, 2025 at 02:20:16PM +0300, Nazir Bilal Yavuz wrote: > Hi, > > On Thu, 9 Jan 2025 at 11:11, Michael Paquier wrote: > > > > On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote: > > > I am a bit confused, are you suggesting these two alternatives: > > > 1- Making pg

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-09 Thread Nazir Bilal Yavuz
Hi, On Thu, 9 Jan 2025 at 11:11, Michael Paquier wrote: > > On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote: > > I am a bit confused, are you suggesting these two alternatives: > > 1- Making pgstat_count_io_op_n() static and continuing to use > > pgstat_count_io_op() as it is. >

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-09 Thread Michael Paquier
On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote: > I am a bit confused, are you suggesting these two alternatives: > 1- Making pgstat_count_io_op_n() static and continuing to use > pgstat_count_io_op() as it is. > 2- Removing pgstat_count_io_op() and instead using > pgstat_count_i

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-09 Thread Nazir Bilal Yavuz
Hi, On Thu, 9 Jan 2025 at 10:15, Nazir Bilal Yavuz wrote: > > On Thu, 9 Jan 2025 at 05:59, Michael Paquier wrote: > > > > > > +static inline bool > > +is_ioop_tracked_in_bytes(IOOp io_op) > > +{ > > +Assert((unsigned int) io_op < IOOP_NUM_TYPES); > > +return io_op >= IOOP_EXTEND; > > +}

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-08 Thread Nazir Bilal Yavuz
Hi, Thanks for the review! On Thu, 9 Jan 2025 at 05:59, Michael Paquier wrote: > > On Thu, Dec 26, 2024 at 02:41:26PM +0300, Nazir Bilal Yavuz wrote: > > Thanks! v4 is attached. I quickly tested the pg_stat_get_backend_io() > > function and it seems it is working. > > Thanks a lot for the rebase

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2025-01-08 Thread Michael Paquier
On Thu, Dec 26, 2024 at 02:41:26PM +0300, Nazir Bilal Yavuz wrote: > Thanks! v4 is attached. I quickly tested the pg_stat_get_backend_io() > function and it seems it is working. Thanks a lot for the rebased version. This looks pretty solid. Here are some comments. void -pgstat_count_io_op(IOOb

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2024-12-26 Thread Nazir Bilal Yavuz
Hi, On Tue, 24 Dec 2024 at 09:13, Michael Paquier wrote: > > On Fri, Dec 06, 2024 at 12:41:55PM +0300, Nazir Bilal Yavuz wrote: > > Thanks! I think 'track' is a better word in this context. I used > > 'tracked in ...', as it sounded more correct to me (I hope it is). > > Splitting op_bytes into t

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2024-12-23 Thread Michael Paquier
On Fri, Dec 06, 2024 at 12:41:55PM +0300, Nazir Bilal Yavuz wrote: > Thanks! I think 'track' is a better word in this context. I used > 'tracked in ...', as it sounded more correct to me (I hope it is). Splitting op_bytes into three fields sounds like a good idea. Count me in regarding the concep

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2024-12-06 Thread Nazir Bilal Yavuz
Hi, On Thu, 5 Dec 2024 at 12:13, Bertrand Drouvot wrote: > > Hi, > > On Wed, Dec 04, 2024 at 02:49:11PM +0300, Nazir Bilal Yavuz wrote: > > On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot > > wrote: > > > > > You are right, no need to have this check; it can not be less than 0. > > I completely r

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2024-12-05 Thread Bertrand Drouvot
Hi, On Wed, Dec 04, 2024 at 02:49:11PM +0300, Nazir Bilal Yavuz wrote: > On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot > wrote: > > > You are right, no need to have this check; it can not be less than 0. > I completely removed the function now. Thanks! Yeah I think that makes sense. > > What

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2024-12-04 Thread Nazir Bilal Yavuz
Hi, Thanks for looking into this! On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot wrote: > > Hi, > > On Wed, Nov 27, 2024 at 11:08:01AM -0500, Melanie Plageman wrote: > > On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz > > wrote: > > > > > > Currently, in the pg_stat_io view, IOs are counted

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2024-11-28 Thread Bertrand Drouvot
Hi, On Wed, Nov 27, 2024 at 11:08:01AM -0500, Melanie Plageman wrote: > On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz wrote: > > > > Currently, in the pg_stat_io view, IOs are counted as blocks. However, > > there are two issues with this approach: > > > > 1- The actual number of IO requests

Re: Make pg_stat_io view count IOs as bytes instead of blocks

2024-11-27 Thread Melanie Plageman
On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz wrote: > > Currently, in the pg_stat_io view, IOs are counted as blocks. However, there > are two issues with this approach: > > 1- The actual number of IO requests to the kernel is lower because IO > requests can be merged before sending the fin