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

At Tue, 12 Jul 2022 12:19:06 -0400, Melanie Plageman <melanieplageman@gmail.com> wrote in 
> > +
> >  &pgStatLocal.shmem->io_ops.stats[backend_type_get_idx(MyBackendType)];
> >
> > backend_type_get_idx(x) is actually (x - 1) plus assertion on the
> > value range. And the only use-case is here. There's an reverse
> > function and also used only at one place.
> >
> > +               Datum           backend_type_desc =
> > +
> >  CStringGetTextDatum(GetBackendTypeDesc(idx_get_backend_type(i)));
> >
> > In this usage GetBackendTypeDesc() gracefully treats out-of-domain
> > values but idx_get_backend_type keenly kills the process for the
> > same. This is inconsistent.
> >
> > My humbel opinion on this is we don't define the two functions and
> > replace the calls to them with (x +/- 1).  Addition to that, I think
> > we should not abort() by invalid backend types.  In that sense, I
> > wonder if we could use B_INVALIDth element for this purpose.
> >
> 
> I think that GetBackendTypeDesc() should probably also error out for an
> unknown value.
> 
> I would be open to not using the helper functions. I thought it would be
> less error-prone, but since it is limited to the code in
> pgstat_io_ops.c, it is probably okay. Let me think a bit more.
> 
> Could you explain more about what you mean about using B_INVALID
> BackendType?

I imagined to use B_INVALID as a kind of "default" partition, which
accepts all unknown backend types. We can just ignore that values but
then we lose the clue for malfunction of stats machinery. I thought
that that backend-type as the sentinel for malfunctions.  Thus we can
emit logs instead.

I feel that the stats machinery shouldn't stop the server as possible,
or I think it is overreaction to abort for invalid values that can be
easily coped with.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center