From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 15 May 2017 03:33:03 +0000 (-0400)
Subject: stats regression test's wait_for_stats() must check timestamp too.
X-Git-Tag: REL_10_BETA1~11
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=eda4ef81511ea62e49f5ea9edb8fbfdd529dd959;p=postgresql

stats regression test's wait_for_stats() must check timestamp too.

pg_stat_get_snapshot_timestamp() returns the timestamp seen in the "global"
stats file.  Because pgstat_write_statsfiles() writes per-DB stats files
before the global file (or at least before renaming it into place), there
is a window where the test backend can see all the stats updates that
wait_for_stats() was checking for (all of which come from the per-DB file)
but also see the same global stats file it had seen at the start of the
test script.  This results in a failure in only the "snapshot_newer" query,
as reported by a couple of buildfarm members recently.

I suspect that this ought to be back-patched.  Commit 4e37b3e15 has
evidently increased the probability of this window getting hit, but
it's not apparent why it could not have been hit before.  I'll refrain
for the moment though.
---

diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index c595442b93..fc91f3ce36 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -32,6 +32,7 @@ declare
   updated1 bool;
   updated2 bool;
   updated3 bool;
+  updated4 bool;
 begin
   -- we don't want to wait forever; loop will exit after 30 seconds
   for i in 1 .. 300 loop
@@ -55,7 +56,13 @@ begin
     SELECT (n_tup_ins > 0) INTO updated3
       FROM pg_stat_user_tables WHERE relname='trunc_stats_test4';
 
-    exit when updated1 and updated2 and updated3;
+    -- We must also check explicitly that pg_stat_get_snapshot_timestamp has
+    -- advanced, because that comes from the global stats file which might
+    -- be older than the per-DB stats file we got the other values from.
+    SELECT (pr.snap_ts < pg_stat_get_snapshot_timestamp()) INTO updated4
+      FROM prevstats AS pr;
+
+    exit when updated1 and updated2 and updated3 and updated4;
 
     -- wait a little
     perform pg_sleep_for('100 milliseconds');
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 026c93038f..6e882bf3ac 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -31,6 +31,7 @@ declare
   updated1 bool;
   updated2 bool;
   updated3 bool;
+  updated4 bool;
 begin
   -- we don't want to wait forever; loop will exit after 30 seconds
   for i in 1 .. 300 loop
@@ -54,7 +55,13 @@ begin
     SELECT (n_tup_ins > 0) INTO updated3
       FROM pg_stat_user_tables WHERE relname='trunc_stats_test4';
 
-    exit when updated1 and updated2 and updated3;
+    -- We must also check explicitly that pg_stat_get_snapshot_timestamp has
+    -- advanced, because that comes from the global stats file which might
+    -- be older than the per-DB stats file we got the other values from.
+    SELECT (pr.snap_ts < pg_stat_get_snapshot_timestamp()) INTO updated4
+      FROM prevstats AS pr;
+
+    exit when updated1 and updated2 and updated3 and updated4;
 
     -- wait a little
     perform pg_sleep_for('100 milliseconds');