At Tue, 12 Jul 2022 12:19:06 -0400, Melanie Plageman <melanieplage...@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