From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Andres Freund <andres(at)anarazel(dot)de> |
Subject: | Re: New instability in stats regression test |
Date: | 2023-11-27 10:19:01 |
Message-ID: | CALj2ACWtgPMg38JTO-AWrdLp4duFzR3iBq39Xt+hn2+S2yzG7w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Nov 27, 2023 at 8:26 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> 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,
It's easy for someone to come and add pg_stat_reset_shared() before
the end without noticing the comment as the test failure is sporadic
in nature.
> 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).
Can running stats.sql in non-parallel mode help stabilize the tests as-is?
> 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?
I tend to agree with this approach, the code is anyways covered. I
think the attached patch also needs to remove setting
archiver_reset_ts (and friends) after pg_stat_reset_shared('archiver')
(and friends), something like [1].
Can we also remove pg_stat_reset_slru() with no argument test to keep
things consistent?
-- Test that multiple SLRUs are reset when no specific SLRU provided
to reset function
SELECT pg_stat_reset_slru();
SELECT stats_reset > :'slru_commit_ts_reset_ts'::timestamptz FROM
pg_stat_slru WHERE name = 'CommitTs';
SELECT stats_reset > :'slru_notify_reset_ts'::timestamptz FROM
pg_stat_slru WHERE name = 'Notify';
[1]
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index d867fb406f..e3b4ca96e8 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -462,37 +462,31 @@ SELECT stats_reset >
:'slru_notify_reset_ts'::timestamptz FROM pg_stat_slru WHER
SELECT stats_reset AS archiver_reset_ts FROM pg_stat_archiver \gset
SELECT pg_stat_reset_shared('archiver');
SELECT stats_reset > :'archiver_reset_ts'::timestamptz FROM pg_stat_archiver;
-SELECT stats_reset AS archiver_reset_ts FROM pg_stat_archiver \gset
-- Test that reset_shared with bgwriter specified as the stats type works
SELECT stats_reset AS bgwriter_reset_ts FROM pg_stat_bgwriter \gset
SELECT pg_stat_reset_shared('bgwriter');
SELECT stats_reset > :'bgwriter_reset_ts'::timestamptz FROM pg_stat_bgwriter;
-SELECT stats_reset AS bgwriter_reset_ts FROM pg_stat_bgwriter \gset
-- Test that reset_shared with checkpointer specified as the stats type works
SELECT stats_reset AS checkpointer_reset_ts FROM pg_stat_checkpointer \gset
SELECT pg_stat_reset_shared('checkpointer');
SELECT stats_reset > :'checkpointer_reset_ts'::timestamptz FROM
pg_stat_checkpointer;
-SELECT stats_reset AS checkpointer_reset_ts FROM pg_stat_checkpointer \gset
-- Test that reset_shared with recovery_prefetch specified as the
stats type works
SELECT stats_reset AS recovery_prefetch_reset_ts FROM
pg_stat_recovery_prefetch \gset
SELECT pg_stat_reset_shared('recovery_prefetch');
SELECT stats_reset > :'recovery_prefetch_reset_ts'::timestamptz FROM
pg_stat_recovery_prefetch;
-SELECT stats_reset AS recovery_prefetch_reset_ts FROM
pg_stat_recovery_prefetch \gset
-- Test that reset_shared with slru specified as the stats type works
SELECT max(stats_reset) AS slru_reset_ts FROM pg_stat_slru \gset
SELECT pg_stat_reset_shared('slru');
SELECT max(stats_reset) > :'slru_reset_ts'::timestamptz FROM pg_stat_slru;
-SELECT max(stats_reset) AS slru_reset_ts FROM pg_stat_slru \gset
-- Test that reset_shared with wal specified as the stats type works
SELECT stats_reset AS wal_reset_ts FROM pg_stat_wal \gset
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 error case for reset_shared with unknown stats type
SELECT pg_stat_reset_shared('unknown');
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2023-11-27 10:34:56 | Re: brininsert optimization opportunity |
Previous Message | Peter Eisentraut | 2023-11-27 10:16:46 | Re: strange para/programlisting pattern in sgml docs |