Re: Add a write_to_file member to PgStat_KindInfo

On Wed, Nov 20, 2024 at 05:13:18PM +0000, Bertrand Drouvot wrote:
> I don't have a strong opinion for this particular case here (I think the code
> is harder to read but yeah there is some code reduction): so I'm fine with
> v2 too.

Well, I like the enthusiasm of having tests, but injection_points
stats being persistent on disk is the only write property I want to
keep in the module, because it can be useful across clean restarts.
Your patch results in a weird mix where you now need to have a total
of four stats IDs rather than two: one more for the fixed-numbered
case and one more for the variable-numbered case to be able to control
the write/no-write property set in shared memory. The correct design,
if we were to control the write/no-write for individual entries, would
be to store a state flag in each individual entries and decide if an
entry should be flushed or not, which would require an additional
callback to check the state of an individual entry at write. Not sure
if we really need that, TBH, until someone comes up with a case for
it.

Your patch adding backend statistics would be a much better fit to add
a test for this specific property, so let's keep injection_points out
of it, even if it means that we would have only coverage for
variable-numbered stats. So let's keep it simple here, and just set
.write_to_file to true for both injection_points stats kinds per the
scope of this thread. Another point is that injection_points acts as
a template for custom pgstats, this makes the code harder to use as a
reference.

Point taken that the tests added in v2 show that the patch works.
--
Michael