postgres.git / summary / log / commit / refs

commit    5a6b95cdf4e9cab086e9278d22fe5952ec7a277b
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Mon Jun 26 20:17:06 2017 +0000

    Ignore old stats file timestamps when starting the stats collector.
    
    The stats collector disregards inquiry messages that bear a cutoff_time
    before when it last wrote the relevant stats file.  That's fine, but at
    startup when it reads the "permanent" stats files, it absorbed their
    timestamps as if they were the times at which the corresponding temporary
    stats files had been written.  In reality, of course, there's no data
    out there at all.  This led to disregarding inquiry messages soon after
    startup if the postmaster had been shut down and restarted within less
    than PGSTAT_STAT_INTERVAL; which is a pretty common scenario, both for
    testing and in the field.  Requesting backends would hang for 10 seconds
    and then report failure to read statistics, unless they got bailed out
    by some other backend coming along and making a newer request within
    that interval.
    
    I came across this through investigating unexpected delays in the
    src/test/recovery TAP tests: it manifests there because the autovacuum
    launcher hangs for 10 seconds when it can't get statistics at startup,
    thus preventing a second shutdown from occurring promptly.  We might
    want to do some things in the autovac code to make it less prone to
    getting stuck that way, but this change is a good bug fix regardless.
    
    In passing, also fix pgstat_read_statsfiles() to ensure that it
    re-zeroes its global stats variables if they are corrupted by a
    short read from the stats file.  (Other reads in that function
    go into temp variables, so that the issue doesn't arise.)
    
    This has been broken since we created the separation between permanent
    and temporary stats files in 8.4, so back-patch to all supported branches.
    
    Discussion: https://postgr.es/m/16860.1498442626@sss.pgh.pa.us


 src/backend/postmaster/pgstat.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index eb9a4fa7ef3..64616a24139 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -4124,9 +4124,20 @@ pgstat_read_statsfiles(Oid onlydb, bool permanent, bool deep)
 	{
 		ereport(pgStatRunningInCollector ? LOG : WARNING,
 				(errmsg("corrupted statistics file \"%s\"", statfile)));
+		memset(&globalStats, 0, sizeof(globalStats));
 		goto done;
 	}
 
+	/*
+	 * In the collector, disregard the timestamp we read from the permanent
+	 * stats file; we should be willing to write a temp stats file immediately
+	 * upon the first request from any backend.  This only matters if the old
+	 * file's timestamp is less than PGSTAT_STAT_INTERVAL ago, but that's not
+	 * an unusual scenario.
+	 */
+	if (pgStatRunningInCollector)
+		globalStats.stats_timestamp = 0;
+
 	/*
 	 * Read archiver stats struct
 	 */
@@ -4134,6 +4145,7 @@ pgstat_read_statsfiles(Oid onlydb, bool permanent, bool deep)
 	{
 		ereport(pgStatRunningInCollector ? LOG : WARNING,
 				(errmsg("corrupted statistics file \"%s\"", statfile)));
+		memset(&archiverStats, 0, sizeof(archiverStats));
 		goto done;
 	}
 
@@ -4178,6 +4190,15 @@ pgstat_read_statsfiles(Oid onlydb, bool permanent, bool deep)
 				dbentry->tables = NULL;
 				dbentry->functions = NULL;
 
+				/*
+				 * In the collector, disregard the timestamp we read from the
+				 * permanent stats file; we should be willing to write a temp
+				 * stats file immediately upon the first request from any
+				 * backend.
+				 */
+				if (pgStatRunningInCollector)
+					dbentry->stats_timestamp = 0;
+
 				/*
 				 * Don't create tables/functions hashtables for uninteresting
 				 * databases.


[parent: d4702b2049c5]