From 4e37b3e15c4e129dbbcf46e00ad36672f4f7b359 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 13 May 2017 09:42:12 -0400 Subject: [PATCH] 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 --- src/test/regress/expected/stats.out | 28 ++++++++++------------------ src/test/regress/sql/stats.sql | 19 ++++++++++--------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index a811265d69..91c061b6c5 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -16,16 +16,8 @@ SET enable_seqscan TO on; SET enable_indexscan TO on; -- for the moment, we don't want index-only scans here SET enable_indexonlyscan TO off; --- wait to let any prior tests finish dumping out stats; --- else our messages might get lost due to contention -SELECT pg_sleep_for('2 seconds'); - pg_sleep_for --------------- - -(1 row) - -- save counters -CREATE TEMP TABLE prevstats AS +CREATE TABLE prevstats AS SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch, (b.heap_blks_read + b.heap_blks_hit) AS heap_blks, (b.idx_blks_read + b.idx_blks_hit) AS idx_blks, @@ -66,7 +58,7 @@ begin exit when updated1 and updated2 and updated3; -- wait a little - perform pg_sleep(0.1); + perform pg_sleep_for('100 milliseconds'); -- reset stats snapshot so we can test again perform pg_stat_clear_snapshot(); @@ -143,14 +135,13 @@ SELECT count(*) FROM tenk2 WHERE unique1 = 1; 1 (1 row) --- force the rate-limiting logic in pgstat_report_stat() to time out --- and send a message -SELECT pg_sleep(1.0); - pg_sleep ----------- - -(1 row) - +-- We can't just call wait_for_stats() at this point, because we only +-- transmit stats when the session goes idle, and we probably didn't +-- transmit the last couple of counts yet thanks to the rate-limiting logic +-- in pgstat_report_stat(). But instead of waiting for the rate limiter's +-- timeout to elapse, let's just start a new session. The old one will +-- then send its stats before dying. +\c - -- wait for stats collector to update SELECT wait_for_stats(); wait_for_stats @@ -199,4 +190,5 @@ FROM prevstats AS pr; (1 row) DROP TABLE trunc_stats_test, trunc_stats_test1, trunc_stats_test2, trunc_stats_test3, trunc_stats_test4; +DROP TABLE prevstats; -- End of Stats Test diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index b3e2efa535..1c21f38008 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -14,12 +14,8 @@ SET enable_indexscan TO on; -- for the moment, we don't want index-only scans here SET enable_indexonlyscan TO off; --- wait to let any prior tests finish dumping out stats; --- else our messages might get lost due to contention -SELECT pg_sleep_for('2 seconds'); - -- save counters -CREATE TEMP TABLE prevstats AS +CREATE TABLE prevstats AS SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch, (b.heap_blks_read + b.heap_blks_hit) AS heap_blks, (b.idx_blks_read + b.idx_blks_hit) AS idx_blks, @@ -61,7 +57,7 @@ begin exit when updated1 and updated2 and updated3; -- wait a little - perform pg_sleep(0.1); + perform pg_sleep_for('100 milliseconds'); -- reset stats snapshot so we can test again perform pg_stat_clear_snapshot(); @@ -137,9 +133,13 @@ SELECT count(*) FROM tenk2; -- do an indexscan SELECT count(*) FROM tenk2 WHERE unique1 = 1; --- force the rate-limiting logic in pgstat_report_stat() to time out --- and send a message -SELECT pg_sleep(1.0); +-- We can't just call wait_for_stats() at this point, because we only +-- transmit stats when the session goes idle, and we probably didn't +-- transmit the last couple of counts yet thanks to the rate-limiting logic +-- in pgstat_report_stat(). But instead of waiting for the rate limiter's +-- timeout to elapse, let's just start a new session. The old one will +-- then send its stats before dying. +\c - -- wait for stats collector to update SELECT wait_for_stats(); @@ -165,4 +165,5 @@ SELECT pr.snap_ts < pg_stat_get_snapshot_timestamp() as snapshot_newer FROM prevstats AS pr; DROP TABLE trunc_stats_test, trunc_stats_test1, trunc_stats_test2, trunc_stats_test3, trunc_stats_test4; +DROP TABLE prevstats; -- End of Stats Test