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

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
> <bertranddrouvot(dot)pg(at)gmail(dot)com> 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 about ordering the enum in IOOp (no bytes/bytes) so that we could check
> > that io_op >= "our firt bytes enum" instead?
> >
> > Also we could create a macro on top of that to make it clear. And a comment
> > would be needed around the IOOp definition.
> >
> > I think that would be simpler to maintain should we add no bytes or bytes op in
> > the future.
>
> I think this is a good idea. I applied all the comments.

Thanks!

+ * non-byte-measured and byte-measured operations. So, that makes is easier
+ * to check whether IO is measured in bytes.

s/that makes is/that makes it/

+ *
+ * Note: If this enum is modified, ensure that both `IOOP_NUM_TYPES` macro
+ * and the `ioop_measured_in_bytes` function are updated accordingly.

Yeah, or?

"Ensure IOOP_EXTEND is the first and IOOP_WRITE the last ones in the measured in
bytes" group and that the groups stay in that order.

That would make the "checks" local to the enum def should someone modify it.

> Created an
> inline function instead of macro and added this 'Assert((unsigned int)
> io_object < IOOBJECT_NUM_TYPES);' to function.

An inline function seems the right choice for me too.

> Done. I moved these checks to the pgstat_count_io_op_n() function. The
> code looks more like its previous version now.

Thanks! Yeah, more easier to follow now.

> > Not sure about "can do IO in bytes" (same wording is used in multiple places).
>
> I changed it to 'measured in bytes' but I am not sure if this is
> better, open to suggestions.

I'm tempted to say something around "track", would mean things like:

1.

ioop_measured_in_bytes => is_ioop_tracked_in_bytes

2.

s/If an op does not do IO in bytes/If an op does not track bytes/

3.

s/not every IO measured in bytes/not every IO tracks bytes/

4.

s/non-byte-measured and byte-measured/non-tracking and tracking bytes/

Thoughts?

Regards,

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