Re: Location for pgstat.stat - Mailing list pgsql-hackers

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Tom Lane wrote:
>>> It doesn't seem to me that it'd be hard to support two locations for the
>>> stats file --- it'd just take another parameter to the read and write
>>> routines.  pgstat.c already knows the difference between a normal write
>>> and a shutdown write ...
>
>> Right. Should it be removed from the permanent location when the server
>> starts?
>
> Yes, I would say so.  There are two possible exit paths: normal shutdown
> (where we'd write a new file) and crash.  In a crash we'd wish to delete
> the file anyway for fear that it's corrupted.
>
>     Startup: read permanent file, then delete it.
>
>     Post-crash: remove any permanent file (same as now)
>
>     Shutdown: write permanent file.
>
>     Normal stats collector write: write temp file.
>
>     Backend stats fetch: read temp file.

Attached is a patch that implements this. I went with the option of just
storing it in a temporary directory that can be symlinked, and not
bothering with a GUC for it. Comments? (documentation updates are also
needed, but I'll wait with those until I hear patch comments :-P)


//Magnus
Index: backend/postmaster/pgstat.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.176
diff -c -r1.176 pgstat.c
*** backend/postmaster/pgstat.c    30 Jun 2008 10:58:47 -0000    1.176
--- backend/postmaster/pgstat.c    4 Aug 2008 09:39:23 -0000
***************
*** 67,74 ****
   * Paths for the statistics files (relative to installation's $PGDATA).
   * ----------
   */
! #define PGSTAT_STAT_FILENAME    "global/pgstat.stat"
! #define PGSTAT_STAT_TMPFILE        "global/pgstat.tmp"

  /* ----------
   * Timer definitions.
--- 67,76 ----
   * Paths for the statistics files (relative to installation's $PGDATA).
   * ----------
   */
! #define PGSTAT_STAT_PERMANENT_FILENAME        "global/pgstat.stat"
! #define PGSTAT_STAT_PERMANENT_TMPFILE        "global/pgstat.tmp"
! #define PGSTAT_STAT_FILENAME                "pgstat_tmp/pgstat.stat"
! #define PGSTAT_STAT_TMPFILE                    "pgstat_tmp/pgstat.tmp"

  /* ----------
   * Timer definitions.
***************
*** 218,225 ****
  static void pgstat_beshutdown_hook(int code, Datum arg);

  static PgStat_StatDBEntry *pgstat_get_db_entry(Oid databaseid, bool create);
! static void pgstat_write_statsfile(void);
! static HTAB *pgstat_read_statsfile(Oid onlydb);
  static void backend_read_statsfile(void);
  static void pgstat_read_current_status(void);

--- 220,227 ----
  static void pgstat_beshutdown_hook(int code, Datum arg);

  static PgStat_StatDBEntry *pgstat_get_db_entry(Oid databaseid, bool create);
! static void pgstat_write_statsfile(bool permanent);
! static HTAB *pgstat_read_statsfile(Oid onlydb, bool permanent);
  static void backend_read_statsfile(void);
  static void pgstat_read_current_status(void);

***************
*** 509,514 ****
--- 511,517 ----
  pgstat_reset_all(void)
  {
      unlink(PGSTAT_STAT_FILENAME);
+     unlink(PGSTAT_STAT_PERMANENT_FILENAME);
  }

  #ifdef EXEC_BACKEND
***************
*** 2595,2601 ****
       * zero.
       */
      pgStatRunningInCollector = true;
!     pgStatDBHash = pgstat_read_statsfile(InvalidOid);

      /*
       * Setup the descriptor set for select(2).    Since only one bit in the set
--- 2598,2604 ----
       * zero.
       */
      pgStatRunningInCollector = true;
!     pgStatDBHash = pgstat_read_statsfile(InvalidOid, true);

      /*
       * Setup the descriptor set for select(2).    Since only one bit in the set
***************
*** 2635,2641 ****
              if (!PostmasterIsAlive(true))
                  break;

!             pgstat_write_statsfile();
              need_statwrite = false;
              need_timer = true;
          }
--- 2638,2644 ----
              if (!PostmasterIsAlive(true))
                  break;

!             pgstat_write_statsfile(false);
              need_statwrite = false;
              need_timer = true;
          }
***************
*** 2803,2809 ****
      /*
       * Save the final stats to reuse at next startup.
       */
!     pgstat_write_statsfile();

      exit(0);
  }
--- 2806,2812 ----
      /*
       * Save the final stats to reuse at next startup.
       */
!     pgstat_write_statsfile(true);

      exit(0);
  }
***************
*** 2891,2897 ****
   * ----------
   */
  static void
! pgstat_write_statsfile(void)
  {
      HASH_SEQ_STATUS hstat;
      HASH_SEQ_STATUS tstat;
--- 2894,2900 ----
   * ----------
   */
  static void
! pgstat_write_statsfile(bool permanent)
  {
      HASH_SEQ_STATUS hstat;
      HASH_SEQ_STATUS tstat;
***************
*** 2901,2917 ****
      PgStat_StatFuncEntry *funcentry;
      FILE       *fpout;
      int32        format_id;

      /*
       * Open the statistics temp file to write out the current values.
       */
!     fpout = fopen(PGSTAT_STAT_TMPFILE, PG_BINARY_W);
      if (fpout == NULL)
      {
          ereport(LOG,
                  (errcode_for_file_access(),
                   errmsg("could not open temporary statistics file \"%s\": %m",
!                         PGSTAT_STAT_TMPFILE)));
          return;
      }

--- 2904,2922 ----
      PgStat_StatFuncEntry *funcentry;
      FILE       *fpout;
      int32        format_id;
+     const char *tmpfile = permanent?PGSTAT_STAT_PERMANENT_TMPFILE:PGSTAT_STAT_TMPFILE;
+     const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:PGSTAT_STAT_FILENAME;

      /*
       * Open the statistics temp file to write out the current values.
       */
!     fpout = fopen(tmpfile, PG_BINARY_W);
      if (fpout == NULL)
      {
          ereport(LOG,
                  (errcode_for_file_access(),
                   errmsg("could not open temporary statistics file \"%s\": %m",
!                         tmpfile)));
          return;
      }

***************
*** 2978,3002 ****
          ereport(LOG,
                  (errcode_for_file_access(),
                 errmsg("could not write temporary statistics file \"%s\": %m",
!                       PGSTAT_STAT_TMPFILE)));
          fclose(fpout);
!         unlink(PGSTAT_STAT_TMPFILE);
      }
      else if (fclose(fpout) < 0)
      {
          ereport(LOG,
                  (errcode_for_file_access(),
                 errmsg("could not close temporary statistics file \"%s\": %m",
!                       PGSTAT_STAT_TMPFILE)));
!         unlink(PGSTAT_STAT_TMPFILE);
      }
!     else if (rename(PGSTAT_STAT_TMPFILE, PGSTAT_STAT_FILENAME) < 0)
      {
          ereport(LOG,
                  (errcode_for_file_access(),
                   errmsg("could not rename temporary statistics file \"%s\" to \"%s\": %m",
!                         PGSTAT_STAT_TMPFILE, PGSTAT_STAT_FILENAME)));
!         unlink(PGSTAT_STAT_TMPFILE);
      }
  }

--- 2983,3007 ----
          ereport(LOG,
                  (errcode_for_file_access(),
                 errmsg("could not write temporary statistics file \"%s\": %m",
!                       tmpfile)));
          fclose(fpout);
!         unlink(tmpfile);
      }
      else if (fclose(fpout) < 0)
      {
          ereport(LOG,
                  (errcode_for_file_access(),
                 errmsg("could not close temporary statistics file \"%s\": %m",
!                       tmpfile)));
!         unlink(tmpfile);
      }
!     else if (rename(tmpfile, statfile) < 0)
      {
          ereport(LOG,
                  (errcode_for_file_access(),
                   errmsg("could not rename temporary statistics file \"%s\" to \"%s\": %m",
!                         tmpfile, statfile)));
!         unlink(tmpfile);
      }
  }

***************
*** 3006,3015 ****
   *
   *    Reads in an existing statistics collector file and initializes the
   *    databases' hash table (whose entries point to the tables' hash tables).
   * ----------
   */
  static HTAB *
! pgstat_read_statsfile(Oid onlydb)
  {
      PgStat_StatDBEntry *dbentry;
      PgStat_StatDBEntry dbbuf;
--- 3011,3025 ----
   *
   *    Reads in an existing statistics collector file and initializes the
   *    databases' hash table (whose entries point to the tables' hash tables).
+  *
+  *    If reading from the permanent file (which happens during collector
+  *    startup, but never from backends), the file is removed once it's been
+  *    successfully read. The temporary file is also removed at this time,
+  *    to make sure backends don't read data from previous runs.
   * ----------
   */
  static HTAB *
! pgstat_read_statsfile(Oid onlydb, bool permanent)
  {
      PgStat_StatDBEntry *dbentry;
      PgStat_StatDBEntry dbbuf;
***************
*** 3024,3029 ****
--- 3034,3040 ----
      FILE       *fpin;
      int32        format_id;
      bool        found;
+     const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:PGSTAT_STAT_FILENAME;

      /*
       * The tables will live in pgStatLocalContext.
***************
*** 3052,3058 ****
       * return zero for anything and the collector simply starts from scratch
       * with empty counters.
       */
!     if ((fpin = AllocateFile(PGSTAT_STAT_FILENAME, PG_BINARY_R)) == NULL)
          return dbhash;

      /*
--- 3063,3069 ----
       * return zero for anything and the collector simply starts from scratch
       * with empty counters.
       */
!     if ((fpin = AllocateFile(statfile, PG_BINARY_R)) == NULL)
          return dbhash;

      /*
***************
*** 3241,3246 ****
--- 3252,3263 ----
  done:
      FreeFile(fpin);

+     if (permanent)
+     {
+         unlink(PGSTAT_STAT_PERMANENT_FILENAME);
+         unlink(PGSTAT_STAT_FILENAME);
+     }
+
      return dbhash;
  }

***************
*** 3259,3267 ****

      /* Autovacuum launcher wants stats about all databases */
      if (IsAutoVacuumLauncherProcess())
!         pgStatDBHash = pgstat_read_statsfile(InvalidOid);
      else
!         pgStatDBHash = pgstat_read_statsfile(MyDatabaseId);
  }


--- 3276,3284 ----

      /* Autovacuum launcher wants stats about all databases */
      if (IsAutoVacuumLauncherProcess())
!         pgStatDBHash = pgstat_read_statsfile(InvalidOid, false);
      else
!         pgStatDBHash = pgstat_read_statsfile(MyDatabaseId, false);
  }


Index: bin/initdb/initdb.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.158
diff -c -r1.158 initdb.c
*** bin/initdb/initdb.c    19 Jul 2008 04:01:29 -0000    1.158
--- bin/initdb/initdb.c    4 Aug 2008 09:39:23 -0000
***************
*** 2461,2467 ****
          "pg_multixact/offsets",
          "base",
          "base/1",
!         "pg_tblspc"
      };

      progname = get_progname(argv[0]);
--- 2461,2468 ----
          "pg_multixact/offsets",
          "base",
          "base/1",
!         "pg_tblspc",
!         "pgstat_tmp"
      };

      progname = get_progname(argv[0]);