Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, melanieplageman(at)gmail(dot)com, vignesh21(at)gmail(dot)com, pryzby(at)telsasoft(dot)com, lukas(at)fittl(dot)com, alvherre(at)alvh(dot)no-ip(dot)org, magnus(at)hagander(dot)net, pgsql-hackers(at)postgresql(dot)org, thomas(dot)munro(at)gmail(dot)com, m(dot)sakrejda(at)gmail(dot)com
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: 2023-02-26 18:20:00
Message-ID: 20520.1677435600@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> Pushed the first (and biggest) commit. More tomorrow.

I hadn't run my buildfarm-compile-warning scraper for a little while,
but I just did, and I find that this commit is causing warnings on
no fewer than 14 buildfarm animals. They all look like

ayu | 2023-02-25 23:02:08 | pgstat_io.c:40:14: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:43:16: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:70:19: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:71:20: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:115:14: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:118:16: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstatfuncs.c:1329:12: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstatfuncs.c:1334:17: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]

That is, these compilers think that comparisons like

io_object < IOOBJECT_NUM_TYPES
io_context < IOCONTEXT_NUM_TYPES

are constant-true. This seems not good; if they were to actually
act on this observation, by removing those loop-ending tests,
we'd have a problem.

The issue seems to be that code like this:

typedef enum IOContext
{
IOCONTEXT_BULKREAD,
IOCONTEXT_BULKWRITE,
IOCONTEXT_NORMAL,
IOCONTEXT_VACUUM,
} IOContext;

#define IOCONTEXT_FIRST IOCONTEXT_BULKREAD
#define IOCONTEXT_NUM_TYPES (IOCONTEXT_VACUUM + 1)

is far too cute for its own good. I'm not sure about how to fix it
either. I thought of defining

#define IOCONTEXT_LAST IOCONTEXT_VACUUM

and make the loop conditions like "io_context <= IOCONTEXT_LAST",
but that doesn't actually fix the problem.

(Even aside from that, I do not find this coding even a little bit
mistake-proof: you still have to remember to update the #define
when adding another enum value.)

We have similar code involving enum ForkNumber but it looks to me
like the loop variables are always declared as plain "int". That
might be the path of least resistance here.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-02-26 18:52:47 Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Previous Message Andres Freund 2023-02-26 18:00:29 Re: stopgap fix for signal handling during restore_command