Re: Make pg_stat_io view count IOs as bytes instead of blocks
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,
Bertrand Drouvot <bertranddrouvot.pg@gmail.com>,
Melanie Plageman <melanieplageman@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-01-15T17:19:03Z
Lists: pgsql-hackers
Nazir Bilal Yavuz <byavuz81@gmail.com> writes:
> On Tue, 14 Jan 2025 at 06:18, Michael Paquier <michael@paquier.xyz> 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
spout a new warning, eg at [1]:
ccache clang -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Werror=unguarded-availability-new -Wendif-labels -Wmissing-format-attribute -Wformat-security -Wmissing-variable-declarations -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -O2 -I. -I. -I../../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c -o pgstat_io.o pgstat_io.c
pgstat_io.c:81:9: warning: comparison of constant 8 with expression of type 'IOOp' (aka 'enum IOOp') is always true [-Wtautological-constant-out-of-range-compare]
Assert(pgstat_is_ioop_tracked_in_bytes(io_op) || bytes == 0);
~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pgstat_io.c:32:11: note: expanded from macro 'pgstat_is_ioop_tracked_in_bytes'
((io_op) < IOOP_NUM_TYPES && (io_op) >= IOOP_EXTEND)
^ ~~~~~~~~~~~~~~
../../../../src/include/c.h:829:9: note: expanded from macro 'Assert'
if (!(condition)) \\
^~~~~~~~~
1 warning generated.
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_tracked_in_bytes(io_op) \
- ((io_op) < IOOP_NUM_TYPES && (io_op) >= IOOP_EXTEND)
+ ((io_op) >= IOOP_EXTEND)
I suppose one alternative is to re-order the enum so that the
upper-limit check in this macro *isn't* tautological ... but
that seems a bit silly.
regards, tom lane
[1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=demoiselle&dt=2025-01-15%2005%3A20%3A59&stg=build
Commits
-
Rework macro pgstat_is_ioop_tracked_in_bytes()
- 7b6468cc9523 18.0 landed
-
Remove assertion in pgstat_count_io_op()
- d2181b321852 18.0 landed
-
Make pg_stat_io count IOs as bytes instead of blocks for some operations
- f92c854cf406 18.0 landed
-
Merge pgstat_count_io_op_n() and pgstat_count_io_op()
- f0bf7857befe 18.0 landed