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
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 7869c598f9..87d881c3f9 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -990,50 +990,6 @@ SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
(1 row)
SELECT stats_reset AS wal_reset_ts FROM pg_stat_wal \gset
--- 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();
- pg_stat_reset_shared
-----------------------
-
-(1 row)
-
-SELECT stats_reset > :'archiver_reset_ts'::timestamptz FROM pg_stat_archiver;
- ?column?
-----------
- t
-(1 row)
-
-SELECT stats_reset > :'bgwriter_reset_ts'::timestamptz FROM pg_stat_bgwriter;
- ?column?
-----------
- t
-(1 row)
-
-SELECT stats_reset > :'checkpointer_reset_ts'::timestamptz FROM pg_stat_checkpointer;
- ?column?
-----------
- t
-(1 row)
-
-SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_recovery_prefetch;
- ?column?
-----------
- t
-(1 row)
-
-SELECT max(stats_reset) > :'slru_reset_ts'::timestamptz FROM pg_stat_slru;
- ?column?
-----------
- t
-(1 row)
-
-SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
- ?column?
-----------
- t
-(1 row)
-
-- Test error case for reset_shared with unknown stats type
SELECT pg_stat_reset_shared('unknown');
ERROR: unrecognized reset target: "unknown"
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 5b4e451a2e..d867fb406f 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -494,16 +494,6 @@ SELECT pg_stat_reset_shared('wal');
SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
SELECT stats_reset AS wal_reset_ts FROM pg_stat_wal \gset
--- 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();
-SELECT stats_reset > :'archiver_reset_ts'::timestamptz FROM pg_stat_archiver;
-SELECT stats_reset > :'bgwriter_reset_ts'::timestamptz FROM pg_stat_bgwriter;
-SELECT stats_reset > :'checkpointer_reset_ts'::timestamptz FROM pg_stat_checkpointer;
-SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM pg_stat_recovery_prefetch;
-SELECT max(stats_reset) > :'slru_reset_ts'::timestamptz FROM pg_stat_slru;
-SELECT stats_reset > :'wal_reset_ts'::timestamptz FROM pg_stat_wal;
-
-- Test error case for reset_shared with unknown stats type
SELECT pg_stat_reset_shared('unknown');
signature.asc
Description: PGP signature
