Skip to content

Commit 4e37b3e

Browse files
committed
Avoid hard-wired sleep delays in stats regression test.
On faster machines, the overall runtime for running the core regression tests is under twenty seconds these days, of which the hard-wired delays in the stats test are a significant fraction. But on closer inspection, it seems like we shouldn't need those. The initial 2-second delay is there only to reduce the risk of the test's stats messages not getting sent due to contention. But analysis of the last ten years' worth of buildfarm runs shows no evidence that such failures actually occur. (We do see failures that look like stats messages not getting sent, particularly on Windows; but there is little reason to believe that the initial delay reduces their frequency.) The later 1-second delay is there to ensure that our session's stats will have gotten sent. But we could also do that by starting a fresh session, which takes well under 1 second even on very slow machines. Hence, let's remove both delays and see what happens. The first delay was the only test of pg_sleep_for() in the regression tests, but we can move that responsibility into wait_for_stats(). Discussion: https://postgr.es/m/17795.1493869423@sss.pgh.pa.us
1 parent 8d9f060 commit 4e37b3e

File tree

2 files changed

+20
-27
lines changed

2 files changed

+20
-27
lines changed

src/test/regress/expected/stats.out

+10-18
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,8 @@ SET enable_seqscan TO on;
1616
SET enable_indexscan TO on;
1717
-- for the moment, we don't want index-only scans here
1818
SET enable_indexonlyscan TO off;
19-
-- wait to let any prior tests finish dumping out stats;
20-
-- else our messages might get lost due to contention
21-
SELECT pg_sleep_for('2 seconds');
22-
pg_sleep_for
23-
--------------
24-
25-
(1 row)
26-
2719
-- save counters
28-
CREATE TEMP TABLE prevstats AS
20+
CREATE TABLE prevstats AS
2921
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
3022
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
3123
(b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
@@ -66,7 +58,7 @@ begin
6658
exit when updated1 and updated2 and updated3;
6759

6860
-- wait a little
69-
perform pg_sleep(0.1);
61+
perform pg_sleep_for('100 milliseconds');
7062

7163
-- reset stats snapshot so we can test again
7264
perform pg_stat_clear_snapshot();
@@ -143,14 +135,13 @@ SELECT count(*) FROM tenk2 WHERE unique1 = 1;
143135
1
144136
(1 row)
145137

146-
-- force the rate-limiting logic in pgstat_report_stat() to time out
147-
-- and send a message
148-
SELECT pg_sleep(1.0);
149-
pg_sleep
150-
----------
151-
152-
(1 row)
153-
138+
-- We can't just call wait_for_stats() at this point, because we only
139+
-- transmit stats when the session goes idle, and we probably didn't
140+
-- transmit the last couple of counts yet thanks to the rate-limiting logic
141+
-- in pgstat_report_stat(). But instead of waiting for the rate limiter's
142+
-- timeout to elapse, let's just start a new session. The old one will
143+
-- then send its stats before dying.
144+
\c -
154145
-- wait for stats collector to update
155146
SELECT wait_for_stats();
156147
wait_for_stats
@@ -199,4 +190,5 @@ FROM prevstats AS pr;
199190
(1 row)
200191

201192
DROP TABLE trunc_stats_test, trunc_stats_test1, trunc_stats_test2, trunc_stats_test3, trunc_stats_test4;
193+
DROP TABLE prevstats;
202194
-- End of Stats Test

src/test/regress/sql/stats.sql

+10-9
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,8 @@ SET enable_indexscan TO on;
1414
-- for the moment, we don't want index-only scans here
1515
SET enable_indexonlyscan TO off;
1616

17-
-- wait to let any prior tests finish dumping out stats;
18-
-- else our messages might get lost due to contention
19-
SELECT pg_sleep_for('2 seconds');
20-
2117
-- save counters
22-
CREATE TEMP TABLE prevstats AS
18+
CREATE TABLE prevstats AS
2319
SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
2420
(b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
2521
(b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
@@ -61,7 +57,7 @@ begin
6157
exit when updated1 and updated2 and updated3;
6258

6359
-- wait a little
64-
perform pg_sleep(0.1);
60+
perform pg_sleep_for('100 milliseconds');
6561

6662
-- reset stats snapshot so we can test again
6763
perform pg_stat_clear_snapshot();
@@ -137,9 +133,13 @@ SELECT count(*) FROM tenk2;
137133
-- do an indexscan
138134
SELECT count(*) FROM tenk2 WHERE unique1 = 1;
139135

140-
-- force the rate-limiting logic in pgstat_report_stat() to time out
141-
-- and send a message
142-
SELECT pg_sleep(1.0);
136+
-- We can't just call wait_for_stats() at this point, because we only
137+
-- transmit stats when the session goes idle, and we probably didn't
138+
-- transmit the last couple of counts yet thanks to the rate-limiting logic
139+
-- in pgstat_report_stat(). But instead of waiting for the rate limiter's
140+
-- timeout to elapse, let's just start a new session. The old one will
141+
-- then send its stats before dying.
142+
\c -
143143

144144
-- wait for stats collector to update
145145
SELECT wait_for_stats();
@@ -165,4 +165,5 @@ SELECT pr.snap_ts < pg_stat_get_snapshot_timestamp() as snapshot_newer
165165
FROM prevstats AS pr;
166166

167167
DROP TABLE trunc_stats_test, trunc_stats_test1, trunc_stats_test2, trunc_stats_test3, trunc_stats_test4;
168+
DROP TABLE prevstats;
168169
-- End of Stats Test

0 commit comments

Comments
 (0)