From bcd0a832586a74d991e32d829ab58f6104624baf Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 5 Apr 2022 19:13:30 -0700 Subject: [PATCH v70 11/27] 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 because pg_stat_statements (and some out-of-core extensions) store data in it. Author: Andres Freund Discussion: https://postgr.es/m/20220330233550.eiwsbearu6xhuqwe@alap3.anarazel.de Discussion: https://postgr.es/m/20220303021600.hs34ghqcw6zcokdh@alap3.anarazel.de --- src/include/pgstat.h | 5 --- src/backend/postmaster/pgstat.c | 12 ------ src/backend/replication/basebackup.c | 36 +--------------- src/backend/utils/misc/guc.c | 41 ------------------- src/backend/utils/misc/postgresql.conf.sample | 1 - src/bin/pg_rewind/filemap.c | 5 +-- .../pg_stat_statements/pg_stat_statements.c | 7 +--- src/test/perl/PostgreSQL/Test/Cluster.pm | 4 -- 8 files changed, 5 insertions(+), 106 deletions(-) diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 666ffcb405d..2f0fb717063 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -644,11 +644,6 @@ extern PGDLLIMPORT bool pgstat_track_counts; extern PGDLLIMPORT int pgstat_track_functions; extern PGDLLIMPORT int pgstat_fetch_consistency; -/* No longer used, but will be removed with GUC */ -extern char *pgstat_stat_directory; -extern char *pgstat_stat_tmpname; -extern char *pgstat_stat_filename; - /* * Variables in pgstat_bgwriter.c diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index e5d77d53186..458d72266eb 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -188,18 +188,6 @@ bool pgstat_track_counts = false; int pgstat_fetch_consistency = PGSTAT_FETCH_CONSISTENCY_NONE; -/* ---------- - * Built from GUC parameter - * ---------- - */ - -char *pgstat_stat_directory = NULL; - -/* No longer used, but will be removed with GUC */ -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 db9fd0a7290..d31b06d885e 100644 --- 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, @@ -239,7 +235,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; @@ -252,8 +247,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(); @@ -281,18 +274,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; @@ -1312,19 +1293,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 bbb156e0dfe..8ff02ef2fb3 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -216,7 +216,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); @@ -4559,17 +4558,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."), @@ -12271,35 +12259,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 5f9a37bed3b..64e5d11cd69 100644 --- 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 7211090f471..1a6c67bdd19 100644 --- 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/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 55786ae84f2..1d6049f2fd8 100644 --- 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/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index b4ebc999356..50e80ae9640 100644 --- 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") -- 2.35.1.677.gabf474a5dd