Date: Wed, 1 Dec 2021 17:59:15 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Andres Freund <andres@anarazel.de>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	Magnus Hagander <magnus@hagander.net>, pgsql-hackers@postgresql.org,
	Lukas Fittl <lukas@fittl.com>,
	Thomas Munro <thomas.munro@gmail.com>
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and
 more?)
Message-ID: <20211201235915.GI17618@telsasoft.com>
References: 
 <CAAKRu_boF3Yh_PeHs10S-88J3fy3AGCQxHCur6+7tgy78YxJ_g@mail.gmail.com>
 <20211008175620.nspjy6wwmzvyzkrg@alap3.anarazel.de>
 <CAAKRu_YJMHJ66-ZB86zFCH=Cq4+5w2gnJ14bu8oyZec0C39Dcg@mail.gmail.com>
 <20211019192931.eeeohw6yrvqe6bol@alap3.anarazel.de>
 <CAAKRu_Yeg+vh6SHNEo1+=O7e-BPX35cU0XQM=YwQRnkFyv_y+w@mail.gmail.com>
 <20211119164952.mlw7ecj4xxliuuut@alap3.anarazel.de>
 <CAAKRu_bCavNULX2BUM1Ankr=Hc5ySbmMJCGnHCFZBbM=yMvFLA@mail.gmail.com>
 <20211125011559.GC17618@telsasoft.com>
 <20211126211636.GE17618@telsasoft.com>
 <CAAKRu_Yrpysob4sftKChKP-zCp3n1pjm+=o824xVp4RutyZ77g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAAKRu_Yrpysob4sftKChKP-zCp3n1pjm+=o824xVp4RutyZ77g@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20211201235915.GI17618%40telsasoft.com>
Precedence: bulk

On Wed, Dec 01, 2021 at 05:00:14PM -0500, Melanie Plageman wrote:
> > Also:
> > src/include/miscadmin.h:#define BACKEND_NUM_TYPES (B_LOGGER + 1)
> >
> > I think it's wrong to say NUM_TYPES = B_LOGGER + 1 (which would suggest using
> > lessthan-or-equal instead of lessthan as you are).
> >
> > Since the valid backend types start at 1 , the "count" of backend types is
> > currently B_LOGGER (13) - not 14.  I think you should remove the "+1" here.
> > Then NROWS (if it continued to exist at all) wouldn't need to subtract one.
> 
> I think what I currently have is technically correct because I start at
> 1 when I am using it as a loop condition. I do waste a spot in the
> arrays I allocate with BACKEND_NUM_TYPES size.
> 
> I was hesitant to make the value of BACKEND_NUM_TYPES == B_LOGGER
> because it seems kind of weird to have it have the same value as the
> B_LOGGER enum.

I don't mean to say that the code is misbehaving - I mean "num_x" means "the
number of x's" - how many there are.  Since the first, valid backend type is 1,
and they're numbered consecutively and without duplicates, then "the number of
backend types" is the same as the value of the last one (B_LOGGER).  It's
confusing if there's a macro called BACKEND_NUM_TYPES which is greater than the
number of backend types.

Most loops say for (int i=0; i<NUM; ++i)
If it's 1-based, they say for (int i=1; i<=NUM; ++i)
You have two different loops like:

+       for (int i = 0; i < BACKEND_NUM_TYPES - 1 ; i++)
+       for (int backend_type = 1; backend_type < BACKEND_NUM_TYPES; backend_type++)

Both of these iterate over the correct number of backend types, but they both
*look* wrong, which isn't desirable.

-- 
Justin