pgstat: remove stats_temp_directory.

With stats now being stored in shared memory, the GUC isn't needed
anymore. However, the pg_stat_tmp directory and PG_STAT_TMP_DIR define are
kept, as pg_stat_statements (and some out-of-core extensions) store data in
it.

Docs will be updated in a subsequent commit, together with the other pending
docs updates due to shared memory stats.

Author: Andres Freund <andres@anarazel.de>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20220330233550.eiwsbearu6xhuqwe@alap3.anarazel.de
Discussion: https://postgr.es/m/20220303021600.hs34ghqcw6zcokdh@alap3.anarazel.de

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 55786ae84f2eeb3fb3b65e4a68642707d160d635..1d6049f2fd887c99e4825b518b98e3a2adf0f2a8 100644 (file)
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -78,12 +78,7 @@ PG_MODULE_MAGIC;
 #define PGSS_DUMP_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pg_stat_statements.stat"
 
 /*
- * Location of external query text file.  We don't keep it in the core
- * system's stats_temp_directory.  The core system can safely use that GUC
- * setting, because the statistics collector temp file paths are set only once
- * as part of changing the GUC, but pg_stat_statements has no way of avoiding
- * race conditions.  Besides, we only expect modest, infrequent I/O for query
- * strings, so placing the file on a faster filesystem is not compelling.
+ * Location of external query text file.
  */
 #define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat"
 

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index a9f3a7ef492f311f47e7d3fe677fa95538b86135..cc6f2700d6f95240188d01cc6c9ce09b03ded8d3 100644 (file)
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -186,16 +186,6 @@ bool               pgstat_track_counts = false;
 int                    pgstat_fetch_consistency = PGSTAT_FETCH_CONSISTENCY_NONE;
 
 
-/* ----------
- * Built from GUC parameter
- * ----------
- */
-
-char      *pgstat_stat_directory = NULL;
-char      *pgstat_stat_filename = NULL;
-char      *pgstat_stat_tmpname = NULL;
-
-
 /* ----------
  * state shared with pgstat_*.c
  * ----------

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index f01fff3a9044ec1fbf9b04850539a9f1e7c22bd0..27e4152446def1443a5ac263d283f903c613d7f8 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -99,9 +99,6 @@ static int    basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
 /* Was the backup currently in-progress initiated in recovery mode? */
 static bool backup_started_in_recovery = false;
 
-/* Relative path of temporary statistics directory */
-static char *statrelpath = NULL;
-
 /* Total number of checksum failures during base backup. */
 static long long int total_checksum_failures;
 
@@ -131,9 +128,8 @@ struct exclude_list_item
 static const char *const excludeDirContents[] =
 {
        /*
-        * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even
-        * when stats_temp_directory is set because PGSS_TEXT_FILE is always
-        * created there.
+        * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
+        * because extensions like pg_stat_statements store data there.
         */
        PG_STAT_TMP_DIR,
 
@@ -237,7 +233,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
        StringInfo      labelfile;
        StringInfo      tblspc_map_file;
        backup_manifest_info manifest;
-       int                     datadirpathlen;
 
        /* Initial backup state, insofar as we know it now. */
        state.tablespaces = NIL;
@@ -250,8 +245,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
        Assert(CurrentResourceOwner == NULL);
        CurrentResourceOwner = ResourceOwnerCreate(NULL, "base backup");
 
-       datadirpathlen = strlen(DataDir);
-
        backup_started_in_recovery = RecoveryInProgress();
 
        labelfile = makeStringInfo();
@@ -279,18 +272,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
                ListCell   *lc;
                tablespaceinfo *ti;
 
-               /*
-                * Calculate the relative path of temporary statistics directory in
-                * order to skip the files which are located in that directory later.
-                */
-               if (is_absolute_path(pgstat_stat_directory) &&
-                       strncmp(pgstat_stat_directory, DataDir, datadirpathlen) == 0)
-                       statrelpath = psprintf("./%s", pgstat_stat_directory + datadirpathlen + 1);
-               else if (strncmp(pgstat_stat_directory, "./", 2) != 0)
-                       statrelpath = psprintf("./%s", pgstat_stat_directory);
-               else
-                       statrelpath = pgstat_stat_directory;
-
                /* Add a node for the base directory at the end */
                ti = palloc0(sizeof(tablespaceinfo));
                ti->size = -1;
@@ -1310,19 +1291,6 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
                if (excludeFound)
                        continue;
 
-               /*
-                * Exclude contents of directory specified by statrelpath if not set
-                * to the default (pg_stat_tmp) which is caught in the loop above.
-                */
-               if (statrelpath != NULL && strcmp(pathbuf, statrelpath) == 0)
-               {
-                       elog(DEBUG1, "contents of directory \"%s\" excluded from backup", statrelpath);
-                       convert_link_to_directory(pathbuf, &statbuf);
-                       size += _tarWriteHeader(sink, pathbuf + basepathlen + 1, NULL,
-                                                                       &statbuf, sizeonly);
-                       continue;
-               }
-
                /*
                 * We can skip pg_wal, the WAL segments need to be fetched from the
                 * WAL archive anyway. But include it as an empty directory anyway, so

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f7758ea4a7b834b7cbafbcf3189112262c458e0f..719f8cb177a4642c1650c3b8e7b15ec403b38fc7 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -217,7 +217,6 @@ static bool check_effective_io_concurrency(int *newval, void **extra, GucSource
 static bool check_maintenance_io_concurrency(int *newval, void **extra, GucSource source);
 static bool check_huge_page_size(int *newval, void **extra, GucSource source);
 static bool check_client_connection_check_interval(int *newval, void **extra, GucSource source);
-static void assign_pgstat_temp_directory(const char *newval, void *extra);
 static bool check_application_name(char **newval, void **extra, GucSource source);
 static void assign_application_name(const char *newval, void *extra);
 static bool check_cluster_name(char **newval, void **extra, GucSource source);
@@ -4560,17 +4559,6 @@ static struct config_string ConfigureNamesString[] =
                NULL, NULL, NULL
        },
 
-       {
-               {"stats_temp_directory", PGC_SIGHUP, STATS_COLLECTOR,
-                       gettext_noop("Writes temporary statistics files to the specified directory."),
-                       NULL,
-                       GUC_SUPERUSER_ONLY
-               },
-               &pgstat_temp_directory,
-               PG_STAT_TMP_DIR,
-               check_canonical_path, assign_pgstat_temp_directory, NULL
-       },
-
        {
                {"synchronous_standby_names", PGC_SIGHUP, REPLICATION_PRIMARY,
                        gettext_noop("Number of synchronous standbys and list of names of potential synchronous ones."),
@@ -12375,35 +12363,6 @@ check_client_connection_check_interval(int *newval, void **extra, GucSource sour
        return true;
 }
 
-static void
-assign_pgstat_temp_directory(const char *newval, void *extra)
-{
-       /* check_canonical_path already canonicalized newval for us */
-       char       *dname;
-       char       *tname;
-       char       *fname;
-
-       /* directory */
-       dname = guc_malloc(ERROR, strlen(newval) + 1);  /* runtime dir */
-       sprintf(dname, "%s", newval);
-
-       /* global stats */
-       tname = guc_malloc(ERROR, strlen(newval) + 12); /* /global.tmp */
-       sprintf(tname, "%s/global.tmp", newval);
-       fname = guc_malloc(ERROR, strlen(newval) + 13); /* /global.stat */
-       sprintf(fname, "%s/global.stat", newval);
-
-       if (pgstat_stat_directory)
-               free(pgstat_stat_directory);
-       pgstat_stat_directory = dname;
-       if (pgstat_stat_tmpname)
-               free(pgstat_stat_tmpname);
-       pgstat_stat_tmpname = tname;
-       if (pgstat_stat_filename)
-               free(pgstat_stat_filename);
-       pgstat_stat_filename = fname;
-}
-
 static bool
 check_application_name(char **newval, void **extra, GucSource source)
 {

diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 5f9a37bed3bf7222e9b8f897cd8144d83998e5a0..64e5d11cd695e7f61089d2e5b49d0da19aaf6cdd 100644 (file)
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -613,7 +613,6 @@
 #track_io_timing = off
 #track_wal_io_timing = off
 #track_functions = none                        # none, pl, all
-#stats_temp_directory = 'pg_stat_tmp'
 #stats_fetch_consistency = none
 
 

diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index fb52debf7a65683d1174dc97b2adda5310726807..d61067f6b2eed6e9b0f33b89e3de9e4974d35eb8 100644 (file)
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -88,9 +88,8 @@ struct exclude_list_item
 static const char *excludeDirContents[] =
 {
        /*
-        * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even
-        * when stats_temp_directory is set because PGSS_TEXT_FILE is always
-        * created there.
+        * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
+        * because extensions like pg_stat_statements store data there.
         */
        "pg_stat_tmp",                          /* defined as PG_STAT_TMP_DIR */
 

diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 1d2d3de86c9640cefc7c73908b6ccd4eb5a0e129..8ac3860e8d4333ae6fbe250df337fe79deac3b9e 100644 (file)
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -642,10 +642,6 @@ extern PGDLLIMPORT bool pgstat_track_counts;
 extern PGDLLIMPORT int pgstat_track_functions;
 extern PGDLLIMPORT int pgstat_fetch_consistency;
 
-extern char *pgstat_stat_directory;
-extern char *pgstat_stat_tmpname;
-extern char *pgstat_stat_filename;
-
 
 /*
  * Variables in pgstat_bgwriter.c

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 1452297210ec6282af551bb2fb456be1b4c68e6e..9a2ada0a103ab48245162fcc5d5350a3c9dc7e76 100644 (file)
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -480,10 +480,6 @@ sub init
        print $conf PostgreSQL::Test::Utils::slurp_file($ENV{TEMP_CONFIG})
          if defined $ENV{TEMP_CONFIG};
 
-       # XXX Neutralize any stats_temp_directory in TEMP_CONFIG.  Nodes running
-       # concurrently must not share a stats_temp_directory.
-       print $conf "stats_temp_directory = 'pg_stat_tmp'\n";
-
        if ($params{allows_streaming})
        {
                if ($params{allows_streaming} eq "logical")