On Sat, Nov 25, 2023 at 02:34:40PM -0500, Tom Lane wrote:
> -- Test that reset_shared with no argument resets all the stats types
> -- supported (providing NULL as argument has the same effect).
> SELECT pg_stat_reset_shared();
Right, this has switched pg_stat_reset_shared() from doing nothing to
do a full reset. Removing this reset switches the results of
io_stats_pre_reset from a 7-digit number to a 4-digit number, thanks
to all the previous I/O activity generated by all the tests.
> Nonetheless, it seems like a really bad idea that this test
> of I/O stats reset happens after the newly-added test. It
> is clearly now dependent on timing and the amount of concurrent
> activity whether it will pass or not. We should probably
> re-order the tests to do the old test first; or else abandon
> this test methodology and just test I/O reset the same way
> we test the other cases (checking only for timestamp advance).
> Or maybe we don't really need the pg_stat_reset_shared() test?
I was ready to argue that we'd better keep this test and keep it close
to the end of stats.sql while documenting why things are kept in this
order, but two resets done on the same shared stats type would still
be prone to race conditions without all the previous activity done in
the tests (like pg_stat_wal).
With all that in mind and because we have checks for the individual
targets with pg_stat_reset_shared(), I would agree to just remove it
entirely. Say as of the attached?
--
Michael