]> granicus.if.org Git - postgresql/commitdiff
Change the pgstat logic so that the stats collector writes the stats file only
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 3 Nov 2008 01:17:08 +0000 (01:17 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 3 Nov 2008 01:17:08 +0000 (01:17 +0000)
upon requests from backends, rather than on a fixed 500msec cycle.  (There's
still throttling logic to ensure it writes no more often than once per
500msec, though.)  This should result in a significant reduction in stats file
write traffic in typical scenarios where the stats are demanded only
infrequently.

This approach also means that the former difficulty with changing
stats_temp_directory on-the-fly has gone away, so remove the caution about
that as well as the thrashing we did to minimize the trouble window.

In passing, also fix pgstat_report_stat() so that we will send a stats
message if we have function call stats but not table stats to report;
this fixes a bug in the recent patch to support function-call stats.

Martin Pihlak

doc/src/sgml/config.sgml
src/backend/postmaster/pgstat.c
src/include/pgstat.h

index dfb976c4731b0b7fa53664db84ddf5b68874c2fc..94fc5ef611fe9ca6d9092ec019853cd001348878 100644 (file)
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.191 2008/09/30 10:52:09 heikki Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.192 2008/11/03 01:17:08 tgl Exp $ -->
 
 <chapter Id="runtime-config">
   <title>Server Configuration</title>
@@ -3347,9 +3347,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         improved performance.
         This parameter can only be set in the <filename>postgresql.conf</>
         file or on the server command line.
-        If this parameter is changed while the server is running, there is a
-        small window of time until the new statistics file has been written
-        during which the statistics functions might return no information.
        </para>
       </listitem>
      </varlistentry>
index 990b10e8ff4a35e1325a63e0600ab1ae67a8545b..d3455cee1924a133afc6caafa2e9f5ab464fe06e 100644 (file)
@@ -13,7 +13,7 @@
  *
  *     Copyright (c) 2001-2008, PostgreSQL Global Development Group
  *
- *     $PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.181 2008/08/25 18:55:43 mha Exp $
+ *     $PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.182 2008/11/03 01:17:08 tgl Exp $
  * ----------
  */
 #include "postgres.h"
  * Timer definitions.
  * ----------
  */
-#define PGSTAT_STAT_INTERVAL   500             /* How often to write the status file;
-                                                                                * in milliseconds. */
+#define PGSTAT_STAT_INTERVAL   500             /* Minimum time between stats file
+                                                                                * updates; in milliseconds. */
+
+#define PGSTAT_RETRY_DELAY             10              /* How long to wait between statistics
+                                                                                * update requests; in milliseconds. */
+
+#define PGSTAT_MAX_WAIT_TIME   5000    /* Maximum time to wait for a stats
+                                                                                * file update; in milliseconds. */
 
 #define PGSTAT_RESTART_INTERVAL 60             /* How often to attempt to restart a
                                                                                 * failed statistics collector; in
@@ -85,6 +91,8 @@
 #define PGSTAT_SELECT_TIMEOUT  2               /* How often to check for postmaster
                                                                                 * death; in seconds. */
 
+#define PGSTAT_POLL_LOOP_COUNT (PGSTAT_MAX_WAIT_TIME / PGSTAT_RETRY_DELAY)
+
 
 /* ----------
  * The initial size hints for the hash tables used in the collector.
@@ -158,6 +166,12 @@ static TabStatusArray *pgStatTabList = NULL;
  */
 static HTAB *pgStatFunctions = NULL;
 
+/*
+ * Indicates if backend has some function stats that it hasn't yet
+ * sent to the collector.
+ */
+static bool have_function_stats = false;
+
 /*
  * Tuple insertion/deletion counts for an open transaction can't be propagated
  * into PgStat_TableStatus counters until we know if it is going to commit
@@ -201,8 +215,12 @@ static int localNumBackends = 0;
  */
 static PgStat_GlobalStats globalStats;
 
+/* Last time the collector successfully wrote the stats file */
+static TimestampTz last_statwrite;
+/* Latest statistics request time from backends */
+static TimestampTz last_statrequest;
+
 static volatile bool need_exit = false;
-static volatile bool need_statwrite = false;
 static volatile bool got_SIGHUP = false;
 
 /*
@@ -223,7 +241,6 @@ static pid_t pgstat_forkexec(void);
 
 NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]);
 static void pgstat_exit(SIGNAL_ARGS);
-static void force_statwrite(SIGNAL_ARGS);
 static void pgstat_beshutdown_hook(int code, Datum arg);
 static void pgstat_sighup_handler(SIGNAL_ARGS);
 
@@ -244,6 +261,7 @@ static void pgstat_setup_memcxt(void);
 static void pgstat_setheader(PgStat_MsgHdr *hdr, StatMsgType mtype);
 static void pgstat_send(void *msg, int len);
 
+static void pgstat_recv_inquiry(PgStat_MsgInquiry *msg, int len);
 static void pgstat_recv_tabstat(PgStat_MsgTabstat *msg, int len);
 static void pgstat_recv_tabpurge(PgStat_MsgTabpurge *msg, int len);
 static void pgstat_recv_dropdb(PgStat_MsgDropdb *msg, int len);
@@ -655,9 +673,8 @@ pgstat_report_stat(bool force)
        int                     i;
 
        /* Don't expend a clock check if nothing to do */
-       /* Note: we ignore pending function stats in this test ... OK? */
-       if (pgStatTabList == NULL ||
-               pgStatTabList->tsa_used == 0)
+       if ((pgStatTabList == NULL || pgStatTabList->tsa_used == 0)
+               && !have_function_stats)
                return;
 
        /*
@@ -823,6 +840,8 @@ pgstat_send_funcstats(void)
        if (msg.m_nentries > 0)
                pgstat_send(&msg, offsetof(PgStat_MsgFuncstat, m_entry[0]) +
                                        msg.m_nentries * sizeof(PgStat_FunctionEntry));
+
+       have_function_stats = false;
 }
 
 
@@ -1242,6 +1261,24 @@ pgstat_ping(void)
        pgstat_send(&msg, sizeof(msg));
 }
 
+/* ----------
+ * pgstat_send_inquiry() -
+ *
+ *     Notify collector that we need fresh data.
+ *     ts specifies the minimum acceptable timestamp for the stats file.
+ * ----------
+ */
+static void
+pgstat_send_inquiry(TimestampTz ts)
+{
+       PgStat_MsgInquiry msg;
+
+       pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_INQUIRY);
+       msg.inquiry_time = ts;
+       pgstat_send(&msg, sizeof(msg));
+}
+
+
 /*
  * Initialize function call usage data.
  * Called by the executor before invoking a function.
@@ -1341,6 +1378,9 @@ pgstat_end_function_usage(PgStat_FunctionCallUsage *fcu, bool finalize)
                fs->f_numcalls++;
        fs->f_time = f_total;
        INSTR_TIME_ADD(fs->f_time_self, f_self);
+
+       /* indicate that we have something to send */
+       have_function_stats = true;
 }
 
 
@@ -2538,8 +2578,6 @@ pgstat_send_bgwriter(void)
 NON_EXEC_STATIC void
 PgstatCollectorMain(int argc, char *argv[])
 {
-       struct itimerval write_timeout;
-       bool            need_timer = false;
        int                     len;
        PgStat_Msg      msg;
 
@@ -2571,13 +2609,13 @@ PgstatCollectorMain(int argc, char *argv[])
 
        /*
         * Ignore all signals usually bound to some action in the postmaster,
-        * except SIGQUIT and SIGALRM.
+        * except SIGQUIT.
         */
        pqsignal(SIGHUP, pgstat_sighup_handler);
        pqsignal(SIGINT, SIG_IGN);
        pqsignal(SIGTERM, SIG_IGN);
        pqsignal(SIGQUIT, pgstat_exit);
-       pqsignal(SIGALRM, force_statwrite);
+       pqsignal(SIGALRM, SIG_IGN);
        pqsignal(SIGPIPE, SIG_IGN);
        pqsignal(SIGUSR1, SIG_IGN);
        pqsignal(SIGUSR2, SIG_IGN);
@@ -2596,12 +2634,8 @@ PgstatCollectorMain(int argc, char *argv[])
        /*
         * Arrange to write the initial status file right away
         */
-       need_statwrite = true;
-
-       /* Preset the delay between status file writes */
-       MemSet(&write_timeout, 0, sizeof(struct itimerval));
-       write_timeout.it_value.tv_sec = PGSTAT_STAT_INTERVAL / 1000;
-       write_timeout.it_value.tv_usec = (PGSTAT_STAT_INTERVAL % 1000) * 1000;
+       last_statrequest = GetCurrentTimestamp();
+       last_statwrite = last_statrequest - 1;
 
        /*
         * Read in an existing statistics stats file or initialize the stats to
@@ -2623,8 +2657,9 @@ PgstatCollectorMain(int argc, char *argv[])
         * death of our parent postmaster.
         *
         * For performance reasons, we don't want to do a PostmasterIsAlive() test
-        * after every message; instead, do it at statwrite time and if
-        * select()/poll() is interrupted by timeout.
+        * after every message; instead, do it only when select()/poll() is
+        * interrupted by timeout.  In essence, we'll stay alive as long as
+        * backends keep sending us stuff often, even if the postmaster is gone.
         */
        for (;;)
        {
@@ -2638,32 +2673,19 @@ PgstatCollectorMain(int argc, char *argv[])
 
                /*
                 * Reload configuration if we got SIGHUP from the postmaster.
-                * Also, signal a new write of the file, so we drop a new file as
-                * soon as possible of the directory for it changes.
                 */
                if (got_SIGHUP)
                {
                        ProcessConfigFile(PGC_SIGHUP);
                        got_SIGHUP = false;
-                       need_statwrite = true;
                }
 
                /*
-                * If time to write the stats file, do so.      Note that the alarm
-                * interrupt isn't re-enabled immediately, but only after we next
-                * receive a stats message; so no cycles are wasted when there is
-                * nothing going on.
+                * Write the stats file if a new request has arrived that is not
+                * satisfied by existing file.
                 */
-               if (need_statwrite)
-               {
-                       /* Check for postmaster death; if so we'll write file below */
-                       if (!PostmasterIsAlive(true))
-                               break;
-
+               if (last_statwrite < last_statrequest)
                        pgstat_write_statsfile(false);
-                       need_statwrite = false;
-                       need_timer = true;
-               }
 
                /*
                 * Wait for a message to arrive; but not for more than
@@ -2756,6 +2778,10 @@ PgstatCollectorMain(int argc, char *argv[])
                                case PGSTAT_MTYPE_DUMMY:
                                        break;
 
+                               case PGSTAT_MTYPE_INQUIRY:
+                                       pgstat_recv_inquiry((PgStat_MsgInquiry *) &msg, len);
+                                       break;
+
                                case PGSTAT_MTYPE_TABSTAT:
                                        pgstat_recv_tabstat((PgStat_MsgTabstat *) &msg, len);
                                        break;
@@ -2800,19 +2826,6 @@ PgstatCollectorMain(int argc, char *argv[])
                                default:
                                        break;
                        }
-
-                       /*
-                        * If this is the first message after we wrote the stats file the
-                        * last time, enable the alarm interrupt to make it be written
-                        * again later.
-                        */
-                       if (need_timer)
-                       {
-                               if (setitimer(ITIMER_REAL, &write_timeout, NULL))
-                                       ereport(ERROR,
-                                       (errmsg("could not set statistics collector timer: %m")));
-                               need_timer = false;
-                       }
                }
                else
                {
@@ -2841,13 +2854,6 @@ pgstat_exit(SIGNAL_ARGS)
        need_exit = true;
 }
 
-/* SIGALRM signal handler for collector process */
-static void
-force_statwrite(SIGNAL_ARGS)
-{
-       need_statwrite = true;
-}
-
 /* SIGHUP handler for collector process */
 static void
 pgstat_sighup_handler(SIGNAL_ARGS)
@@ -2943,7 +2949,7 @@ pgstat_write_statsfile(bool permanent)
        /*
         * Open the statistics temp file to write out the current values.
         */
-       fpout = fopen(tmpfile, PG_BINARY_W);
+       fpout = AllocateFile(tmpfile, PG_BINARY_W);
        if (fpout == NULL)
        {
                ereport(LOG,
@@ -2953,6 +2959,11 @@ pgstat_write_statsfile(bool permanent)
                return;
        }
 
+       /*
+        * Set the timestamp of the stats file.
+        */
+       globalStats.stats_timestamp = GetCurrentTimestamp();
+
        /*
         * Write the file header --- currently just a format ID.
         */
@@ -3017,10 +3028,10 @@ pgstat_write_statsfile(bool permanent)
                                (errcode_for_file_access(),
                           errmsg("could not write temporary statistics file \"%s\": %m",
                                          tmpfile)));
-               fclose(fpout);
+               FreeFile(fpout);
                unlink(tmpfile);
        }
-       else if (fclose(fpout) < 0)
+       else if (FreeFile(fpout) < 0)
        {
                ereport(LOG,
                                (errcode_for_file_access(),
@@ -3036,6 +3047,22 @@ pgstat_write_statsfile(bool permanent)
                                                tmpfile, statfile)));
                unlink(tmpfile);
        }
+       else
+       {
+               /*
+                * Successful write, so update last_statwrite.
+                */
+               last_statwrite = globalStats.stats_timestamp;
+
+               /*
+                * It's not entirely clear whether there could be clock skew between
+                * backends and the collector; but just in case someone manages to
+                * send us a stats request time that's far in the future, reset it.
+                * This ensures that no inquiry message can cause more than one stats
+                * file write to occur.
+                */
+               last_statrequest = last_statwrite;
+       }
 
        if (permanent)
                unlink(pgstat_stat_filename);
@@ -3289,6 +3316,52 @@ done:
        return dbhash;
 }
 
+/* ----------
+ * pgstat_read_statsfile_timestamp() -
+ *
+ *     Attempt to fetch the timestamp of an existing stats file.
+ *     Returns TRUE if successful (timestamp is stored at *ts).
+ * ----------
+ */
+static bool
+pgstat_read_statsfile_timestamp(bool permanent, TimestampTz *ts)
+{
+       PgStat_GlobalStats myGlobalStats;
+       FILE       *fpin;
+       int32           format_id;
+       const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:pgstat_stat_filename;
+
+       /*
+        * Try to open the status file.
+        */
+       if ((fpin = AllocateFile(statfile, PG_BINARY_R)) == NULL)
+               return false;
+
+       /*
+        * Verify it's of the expected format.
+        */
+       if (fread(&format_id, 1, sizeof(format_id), fpin) != sizeof(format_id)
+               || format_id != PGSTAT_FILE_FORMAT_ID)
+       {
+               FreeFile(fpin);
+               return false;
+       }
+
+       /*
+        * Read global stats struct
+        */
+       if (fread(&myGlobalStats, 1, sizeof(myGlobalStats), fpin) != sizeof(myGlobalStats))
+       {
+               FreeFile(fpin);
+               return false;
+       }
+
+       *ts = myGlobalStats.stats_timestamp;
+
+       FreeFile(fpin);
+       return true;
+}
+
 /*
  * If not already done, read the statistics collector stats file into
  * some hash tables.  The results will be kept until pgstat_clear_snapshot()
@@ -3297,11 +3370,50 @@ done:
 static void
 backend_read_statsfile(void)
 {
+       TimestampTz min_ts;
+       int                     count;
+
        /* already read it? */
        if (pgStatDBHash)
                return;
        Assert(!pgStatRunningInCollector);
 
+       /*
+        * We set the minimum acceptable timestamp to PGSTAT_STAT_INTERVAL msec
+        * before now.  This indirectly ensures that the collector needn't write
+        * the file more often than PGSTAT_STAT_INTERVAL.
+        *
+        * Note that we don't recompute min_ts after sleeping; so we might end up
+        * accepting a file a bit older than PGSTAT_STAT_INTERVAL.  In practice
+        * that shouldn't happen, though, as long as the sleep time is less than
+        * PGSTAT_STAT_INTERVAL; and we don't want to lie to the collector about
+        * what our cutoff time really is.
+        */
+       min_ts = TimestampTzPlusMilliseconds(GetCurrentTimestamp(),
+                                                                                -PGSTAT_STAT_INTERVAL);
+
+       /*
+        * Loop until fresh enough stats file is available or we ran out of time.
+        * The stats inquiry message is sent repeatedly in case collector drops it.
+        */
+       for (count = 0; count < PGSTAT_POLL_LOOP_COUNT; count++)
+       {
+               TimestampTz file_ts;
+
+               CHECK_FOR_INTERRUPTS();
+
+               if (pgstat_read_statsfile_timestamp(false, &file_ts) &&
+                       file_ts >= min_ts)
+                       break;
+
+               /* Not there or too old, so kick the collector and wait a bit */
+               pgstat_send_inquiry(min_ts);
+               pg_usleep(PGSTAT_RETRY_DELAY * 1000L);
+       }
+
+       if (count >= PGSTAT_POLL_LOOP_COUNT)
+               elog(WARNING, "pgstat wait timeout");
+
        /* Autovacuum launcher wants stats about all databases */
        if (IsAutoVacuumLauncherProcess())
                pgStatDBHash = pgstat_read_statsfile(InvalidOid, false);
@@ -3353,6 +3465,20 @@ pgstat_clear_snapshot(void)
 }
 
 
+/* ----------
+ * pgstat_recv_inquiry() -
+ *
+ *     Process stat inquiry requests.
+ * ----------
+ */
+static void
+pgstat_recv_inquiry(PgStat_MsgInquiry *msg, int len)
+{
+       if (msg->inquiry_time > last_statrequest)
+               last_statrequest = msg->inquiry_time;
+}
+
+
 /* ----------
  * pgstat_recv_tabstat() -
  *
index 8c2d39e4c5533bbf3364e9a47c64044f7976bbdd..119d7b6966d5be8bcfb956835dab530e8843bc73 100644 (file)
@@ -5,7 +5,7 @@
  *
  *     Copyright (c) 2001-2008, PostgreSQL Global Development Group
  *
- *     $PostgreSQL: pgsql/src/include/pgstat.h,v 1.78 2008/08/15 08:37:40 mha Exp $
+ *     $PostgreSQL: pgsql/src/include/pgstat.h,v 1.79 2008/11/03 01:17:08 tgl Exp $
  * ----------
  */
 #ifndef PGSTAT_H
@@ -33,6 +33,7 @@ typedef enum TrackFunctionsLevel
 typedef enum StatMsgType
 {
        PGSTAT_MTYPE_DUMMY,
+       PGSTAT_MTYPE_INQUIRY,
        PGSTAT_MTYPE_TABSTAT,
        PGSTAT_MTYPE_TABPURGE,
        PGSTAT_MTYPE_DROPDB,
@@ -173,6 +174,19 @@ typedef struct PgStat_MsgDummy
 } PgStat_MsgDummy;
 
 
+/* ----------
+ * PgStat_MsgInquiry                   Sent by a backend to ask the collector
+ *                                                             to write the stats file.
+ * ----------
+ */
+
+typedef struct PgStat_MsgInquiry
+{
+       PgStat_MsgHdr   m_hdr;
+       TimestampTz             inquiry_time;   /* minimum acceptable file timestamp */
+} PgStat_MsgInquiry;
+
+
 /* ----------
  * PgStat_TableEntry                   Per-table info in a MsgTabstat
  * ----------
@@ -392,6 +406,7 @@ typedef union PgStat_Msg
 {
        PgStat_MsgHdr msg_hdr;
        PgStat_MsgDummy msg_dummy;
+       PgStat_MsgInquiry msg_inquiry;
        PgStat_MsgTabstat msg_tabstat;
        PgStat_MsgTabpurge msg_tabpurge;
        PgStat_MsgDropdb msg_dropdb;
@@ -413,7 +428,7 @@ typedef union PgStat_Msg
  * ------------------------------------------------------------
  */
 
-#define PGSTAT_FILE_FORMAT_ID  0x01A5BC97
+#define PGSTAT_FILE_FORMAT_ID  0x01A5BC98
 
 /* ----------
  * PgStat_StatDBEntry                  The collector's data per database
@@ -494,6 +509,7 @@ typedef struct PgStat_StatFuncEntry
  */
 typedef struct PgStat_GlobalStats
 {
+       TimestampTz stats_timestamp;            /* time of stats file update */
        PgStat_Counter timed_checkpoints;
        PgStat_Counter requested_checkpoints;
        PgStat_Counter buf_written_checkpoints;