Horton

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

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

View thread View on postgr.es
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: melanieplageman@gmail.com
Cc: andres@anarazel.de, pryzby@telsasoft.com, alvherre@alvh.no-ip.org, magnus@hagander.net, pgsql-hackers@postgresql.org, lukas@fittl.com, thomas.munro@gmail.com
Date: 2022-07-13T02:00:07Z
Lists: pgsql-hackers 
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

Commits

  1. Stabilize pg_stat_io writes test

  2. Fix flakey pg_stat_io test

  3. Suppress more compiler warnings in new pgstats code.

  4. Suppress compiler warnings in new pgstats code.

  5. Add tests for pg_stat_io

  6. Create regress_tblspc in test_setup

  7. Add pg_stat_io view, providing more detailed IO statistics

  8. pgstat: Track more detailed relation IO statistics

  9. pgstat: Infrastructure for more detailed IO statistics

  10. doc: Fix some issues in logical replication section

  11. Manual cleanup and pgindent of pgstat and bufmgr related code

  12. Have the planner consider Incremental Sort for DISTINCT

  13. Use actual backend IDs in pg_stat_get_backend_idset() and friends.

  14. Remove redundant call to pgstat_report_wal()

  15. Add BackendType for standalone backends

  16. Initialize backend status reporting during bootstrap.