Re: New instability in stats regression test
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org, Andres Freund <andres@anarazel.de>
Date: 2023-11-27T02:56:19Z
Lists: pgsql-hackers
Attachments
- stats-test.patch (text/x-diff) patch
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
Commits
-
Remove test for pg_stat_reset_shared() in stats.sql
- a9a8108411e4 17.0 landed
-
Add ability to reset all shared stats types in pg_stat_reset_shared()
- 23c8c0c8f472 17.0 cited
-
Add tests for pg_stat_io
- 10a082bf7215 16.0 cited