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

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 <michael(at)paquier(dot)xyz> wrote:
> >
> > On Thu, Jan 09, 2025 at 03:30:37PM +0000, 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?
> > >
> > > #define is_ioop_tracked_in_bytes(io_op) \
> > > ((io_op) < IOOP_NUM_TYPES && (io_op) >= IOOP_EXTEND)
> >
> > Indeed. Your suggestion to use a macro makes more sense to me because
> > is_ioop_tracked_in_bytes() itself has an Assert(), while being itself
> > only used in an assertion, as you say. Better to document the
> > dependency on the ordering of IOOp, even if that's kind of hard to
> > miss.
>
> 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.

Yeah, I think we can remove the "io_op < IOOP_NUM_TYPE" assertion in
pgstat_count_io_op() (but keep this check as part of the macro).

> > That
> > seems OK here. Please let me know if you have more comments.

Appart from the above, LGTM.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com