Re: temporary statistics option at initdb time - Mailing list pgsql-hackers

Magnus Hagander wrote:
> Tom Lane wrote:
>> Decibel! <decibel@decibel.org> writes:
>>> I disagree. While we don't guarantee stats are absolutely up-to-date,
>>> or atomic I don't think that gives license for them to just magically
>>> not exist sometimes.
>>> Would it really be that hard to have the system copy the file out
>>> before telling all the other backends of the change?
>> Well, there is no (zero, zilch, nada) use-case for changing this setting
>> on the fly.  Why not make it a "frozen at postmaster start" GUC?  Seems
>> like that gets all the functionality needed and most of the ease of use.
>
> Oh, there is a use-case. If you run your system and then only afterwards
> realize the I/O from the stats file is high enough to be an issue, and
> want to change it.
>
> That said, I'm not sure the use-case is anywhere near common enough to
> put a lot of code into it.
>
> But I can certainly look at making it a startup GUC. As you say, that'll
> solve *most* of the cases.

Here's a patch that implements the simple case making it a
PGC_POSTMASTER variable. Is this good enough for people? ;-)

//Magnus

Index: src/backend/postmaster/pgstat.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.178
diff -c -r1.178 pgstat.c
*** src/backend/postmaster/pgstat.c    5 Aug 2008 12:09:30 -0000    1.178
--- src/backend/postmaster/pgstat.c    13 Aug 2008 13:32:02 -0000
***************
*** 70,77 ****
   */
  #define PGSTAT_STAT_PERMANENT_FILENAME        "global/pgstat.stat"
  #define PGSTAT_STAT_PERMANENT_TMPFILE        "global/pgstat.tmp"
- #define PGSTAT_STAT_FILENAME                "pg_stat_tmp/pgstat.stat"
- #define PGSTAT_STAT_TMPFILE                    "pg_stat_tmp/pgstat.tmp"

  /* ----------
   * Timer definitions.
--- 70,75 ----
***************
*** 106,111 ****
--- 104,116 ----
  int            pgstat_track_functions = TRACK_FUNC_OFF;
  int            pgstat_track_activity_query_size = 1024;

+ /* ----------
+  * Built from GUC parameter
+  * ----------
+  */
+ char       *pgstat_stat_filename = NULL;
+ char       *pgstat_stat_tmpname = NULL;
+
  /*
   * BgWriter global statistics counters (unused in other processes).
   * Stored directly in a stats message structure so it can be sent
***************
*** 511,517 ****
  void
  pgstat_reset_all(void)
  {
!     unlink(PGSTAT_STAT_FILENAME);
      unlink(PGSTAT_STAT_PERMANENT_FILENAME);
  }

--- 516,522 ----
  void
  pgstat_reset_all(void)
  {
!     unlink(pgstat_stat_filename);
      unlink(PGSTAT_STAT_PERMANENT_FILENAME);
  }

***************
*** 2911,2918 ****
      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.
--- 2916,2923 ----
      PgStat_StatFuncEntry *funcentry;
      FILE       *fpout;
      int32        format_id;
!     const char *tmpfile = permanent?PGSTAT_STAT_PERMANENT_TMPFILE:pgstat_stat_tmpname;
!     const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:pgstat_stat_filename;

      /*
       * Open the statistics temp file to write out the current values.
***************
*** 3012,3018 ****
      }

      if (permanent)
!         unlink(PGSTAT_STAT_FILENAME);
  }


--- 3017,3023 ----
      }

      if (permanent)
!         unlink(pgstat_stat_filename);
  }


***************
*** 3039,3045 ****
      FILE       *fpin;
      int32        format_id;
      bool        found;
!     const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:PGSTAT_STAT_FILENAME;

      /*
       * The tables will live in pgStatLocalContext.
--- 3044,3050 ----
      FILE       *fpin;
      int32        format_id;
      bool        found;
!     const char *statfile = permanent?PGSTAT_STAT_PERMANENT_FILENAME:pgstat_stat_filename;

      /*
       * The tables will live in pgStatLocalContext.
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.465
diff -c -r1.465 guc.c
*** src/backend/utils/misc/guc.c    23 Jul 2008 17:29:53 -0000    1.465
--- src/backend/utils/misc/guc.c    13 Aug 2008 13:32:03 -0000
***************
*** 164,169 ****
--- 164,170 ----
  static const char *show_tcp_keepalives_count(void);
  static bool assign_autovacuum_max_workers(int newval, bool doit, GucSource source);
  static bool assign_maxconnections(int newval, bool doit, GucSource source);
+ static const char *assign_pgstat_temp_directory(const char *newval, bool doit, GucSource source);

  static char *config_enum_get_options(struct config_enum *record,
                                       const char *prefix, const char *suffix);
***************
*** 343,348 ****
--- 344,351 ----
  char       *IdentFileName;
  char       *external_pid_file;

+ char       *pgstat_temp_directory;
+
  int            tcp_keepalives_idle;
  int            tcp_keepalives_interval;
  int            tcp_keepalives_count;
***************
*** 2467,2472 ****
--- 2470,2485 ----
      },

      {
+         {"stats_temp_directory", PGC_POSTMASTER, STATS_COLLECTOR,
+             gettext_noop("Writes temporary statistics files to the specified directory."),
+             NULL,
+             GUC_SUPERUSER_ONLY
+         },
+         &pgstat_temp_directory,
+         "pg_stat_tmp", assign_pgstat_temp_directory, NULL
+     },
+
+     {
          {"default_text_search_config", PGC_USERSET, CLIENT_CONN_LOCALE,
              gettext_noop("Sets default text search configuration."),
              NULL
***************
*** 7370,7373 ****
--- 7383,7406 ----
      return true;
  }

+ static const char *
+ assign_pgstat_temp_directory(const char *newval, bool doit, GucSource source)
+ {
+     if (doit)
+     {
+         if (pgstat_stat_tmpname)
+             free(pgstat_stat_tmpname);
+         if (pgstat_stat_filename)
+             free(pgstat_stat_filename);
+
+         pgstat_stat_tmpname = guc_malloc(FATAL, strlen(newval) + 12);  /* /pgstat.tmp */
+         pgstat_stat_filename = guc_malloc(FATAL, strlen(newval) + 13); /* /pgstat.stat */
+
+         sprintf(pgstat_stat_tmpname, "%s/pgstat.tmp", newval);
+         sprintf(pgstat_stat_filename, "%s/pgstat.stat", newval);
+     }
+
+     return newval;
+ }
+
  #include "guc-file.c"
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.243
diff -c -r1.243 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample    30 Jun 2008 10:58:47 -0000    1.243
--- src/backend/utils/misc/postgresql.conf.sample    13 Aug 2008 13:32:03 -0000
***************
*** 366,371 ****
--- 366,372 ----
  #track_functions = none            # none, pl, all
  #track_activity_query_size = 1024
  #update_process_title = on
+ #stats_temp_directory = 'pg_stat_tmp'


  # - Statistics Monitoring -
Index: src/include/pgstat.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/pgstat.h,v
retrieving revision 1.77
diff -c -r1.77 pgstat.h
*** src/include/pgstat.h    30 Jun 2008 10:58:47 -0000    1.77
--- src/include/pgstat.h    13 Aug 2008 13:32:03 -0000
***************
*** 576,581 ****
--- 576,583 ----
  extern bool pgstat_track_counts;
  extern int    pgstat_track_functions;
  extern int    pgstat_track_activity_query_size;
+ extern char *pgstat_stat_tmpname;
+ extern char *pgstat_stat_filename;

  /*
   * BgWriter statistics counters are updated directly by bgwriter and bufmgr