Skip to content

Commit 6f0cf87

Browse files
anarazelanarazel
committed
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
1 parent 5891c7a commit 6f0cf87

File tree

8 files changed

+5
-103
lines changed

8 files changed

+5
-103
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,7 @@ PG_MODULE_MAGIC;
7878
#define PGSS_DUMP_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pg_stat_statements.stat"
7979

8080
/*
81-
* Location of external query text file. We don't keep it in the core
82-
* system's stats_temp_directory. The core system can safely use that GUC
83-
* setting, because the statistics collector temp file paths are set only once
84-
* as part of changing the GUC, but pg_stat_statements has no way of avoiding
85-
* race conditions. Besides, we only expect modest, infrequent I/O for query
86-
* strings, so placing the file on a faster filesystem is not compelling.
81+
* Location of external query text file.
8782
*/
8883
#define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat"
8984

‎src/backend/postmaster/pgstat.c‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -186,16 +186,6 @@ bool pgstat_track_counts = false;
186186
int pgstat_fetch_consistency = PGSTAT_FETCH_CONSISTENCY_NONE;
187187

188188

189-
/* ----------
190-
* Built from GUC parameter
191-
* ----------
192-
*/
193-
194-
char *pgstat_stat_directory = NULL;
195-
char *pgstat_stat_filename = NULL;
196-
char *pgstat_stat_tmpname = NULL;
197-
198-
199189
/* ----------
200190
* state shared with pgstat_*.c
201191
* ----------

‎src/backend/replication/basebackup.c‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,6 @@ static int basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
9999
/* Was the backup currently in-progress initiated in recovery mode? */
100100
static bool backup_started_in_recovery = false;
101101

102-
/* Relative path of temporary statistics directory */
103-
static char *statrelpath = NULL;
104-
105102
/* Total number of checksum failures during base backup. */
106103
static long long int total_checksum_failures;
107104

@@ -131,9 +128,8 @@ struct exclude_list_item
131128
static const char *const excludeDirContents[] =
132129
{
133130
/*
134-
* Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even
135-
* when stats_temp_directory is set because PGSS_TEXT_FILE is always
136-
* created there.
131+
* Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
132+
* because extensions like pg_stat_statements store data there.
137133
*/
138134
PG_STAT_TMP_DIR,
139135

@@ -237,7 +233,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
237233
StringInfo labelfile;
238234
StringInfo tblspc_map_file;
239235
backup_manifest_info manifest;
240-
int datadirpathlen;
241236

242237
/* Initial backup state, insofar as we know it now. */
243238
state.tablespaces = NIL;
@@ -250,8 +245,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
250245
Assert(CurrentResourceOwner == NULL);
251246
CurrentResourceOwner = ResourceOwnerCreate(NULL, "base backup");
252247

253-
datadirpathlen = strlen(DataDir);
254-
255248
backup_started_in_recovery = RecoveryInProgress();
256249

257250
labelfile = makeStringInfo();
@@ -279,18 +272,6 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
279272
ListCell *lc;
280273
tablespaceinfo *ti;
281274

282-
/*
283-
* Calculate the relative path of temporary statistics directory in
284-
* order to skip the files which are located in that directory later.
285-
*/
286-
if (is_absolute_path(pgstat_stat_directory) &&
287-
strncmp(pgstat_stat_directory, DataDir, datadirpathlen) == 0)
288-
statrelpath = psprintf("./%s", pgstat_stat_directory + datadirpathlen + 1);
289-
else if (strncmp(pgstat_stat_directory, "./", 2) != 0)
290-
statrelpath = psprintf("./%s", pgstat_stat_directory);
291-
else
292-
statrelpath = pgstat_stat_directory;
293-
294275
/* Add a node for the base directory at the end */
295276
ti = palloc0(sizeof(tablespaceinfo));
296277
ti->size = -1;
@@ -1310,19 +1291,6 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
13101291
if (excludeFound)
13111292
continue;
13121293

1313-
/*
1314-
* Exclude contents of directory specified by statrelpath if not set
1315-
* to the default (pg_stat_tmp) which is caught in the loop above.
1316-
*/
1317-
if (statrelpath != NULL && strcmp(pathbuf, statrelpath) == 0)
1318-
{
1319-
elog(DEBUG1, "contents of directory \"%s\" excluded from backup", statrelpath);
1320-
convert_link_to_directory(pathbuf, &statbuf);
1321-
size += _tarWriteHeader(sink, pathbuf + basepathlen + 1, NULL,
1322-
&statbuf, sizeonly);
1323-
continue;
1324-
}
1325-
13261294
/*
13271295
* We can skip pg_wal, the WAL segments need to be fetched from the
13281296
* WAL archive anyway. But include it as an empty directory anyway, so

‎src/backend/utils/misc/guc.c‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,6 @@ static bool check_effective_io_concurrency(int *newval, void **extra, GucSource
217217
static bool check_maintenance_io_concurrency(int *newval, void **extra, GucSource source);
218218
static bool check_huge_page_size(int *newval, void **extra, GucSource source);
219219
static bool check_client_connection_check_interval(int *newval, void **extra, GucSource source);
220-
static void assign_pgstat_temp_directory(const char *newval, void *extra);
221220
static bool check_application_name(char **newval, void **extra, GucSource source);
222221
static void assign_application_name(const char *newval, void *extra);
223222
static bool check_cluster_name(char **newval, void **extra, GucSource source);
@@ -4560,17 +4559,6 @@ static struct config_string ConfigureNamesString[] =
45604559
NULL, NULL, NULL
45614560
},
45624561

4563-
{
4564-
{"stats_temp_directory", PGC_SIGHUP, STATS_COLLECTOR,
4565-
gettext_noop("Writes temporary statistics files to the specified directory."),
4566-
NULL,
4567-
GUC_SUPERUSER_ONLY
4568-
},
4569-
&pgstat_temp_directory,
4570-
PG_STAT_TMP_DIR,
4571-
check_canonical_path, assign_pgstat_temp_directory, NULL
4572-
},
4573-
45744562
{
45754563
{"synchronous_standby_names", PGC_SIGHUP, REPLICATION_PRIMARY,
45764564
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
1237512363
return true;
1237612364
}
1237712365

12378-
static void
12379-
assign_pgstat_temp_directory(const char *newval, void *extra)
12380-
{
12381-
/* check_canonical_path already canonicalized newval for us */
12382-
char *dname;
12383-
char *tname;
12384-
char *fname;
12385-
12386-
/* directory */
12387-
dname = guc_malloc(ERROR, strlen(newval) + 1); /* runtime dir */
12388-
sprintf(dname, "%s", newval);
12389-
12390-
/* global stats */
12391-
tname = guc_malloc(ERROR, strlen(newval) + 12); /* /global.tmp */
12392-
sprintf(tname, "%s/global.tmp", newval);
12393-
fname = guc_malloc(ERROR, strlen(newval) + 13); /* /global.stat */
12394-
sprintf(fname, "%s/global.stat", newval);
12395-
12396-
if (pgstat_stat_directory)
12397-
free(pgstat_stat_directory);
12398-
pgstat_stat_directory = dname;
12399-
if (pgstat_stat_tmpname)
12400-
free(pgstat_stat_tmpname);
12401-
pgstat_stat_tmpname = tname;
12402-
if (pgstat_stat_filename)
12403-
free(pgstat_stat_filename);
12404-
pgstat_stat_filename = fname;
12405-
}
12406-
1240712366
static bool
1240812367
check_application_name(char **newval, void **extra, GucSource source)
1240912368
{

‎src/backend/utils/misc/postgresql.conf.sample‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,6 @@
613613
#track_io_timing = off
614614
#track_wal_io_timing = off
615615
#track_functions = none # none, pl, all
616-
#stats_temp_directory = 'pg_stat_tmp'
617616
#stats_fetch_consistency = none
618617

619618

‎src/bin/pg_rewind/filemap.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,8 @@ struct exclude_list_item
8888
static const char *excludeDirContents[] =
8989
{
9090
/*
91-
* Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even
92-
* when stats_temp_directory is set because PGSS_TEXT_FILE is always
93-
* created there.
91+
* Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
92+
* because extensions like pg_stat_statements store data there.
9493
*/
9594
"pg_stat_tmp", /* defined as PG_STAT_TMP_DIR */
9695

‎src/include/pgstat.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -642,10 +642,6 @@ extern PGDLLIMPORT bool pgstat_track_counts;
642642
extern PGDLLIMPORT int pgstat_track_functions;
643643
extern PGDLLIMPORT int pgstat_fetch_consistency;
644644

645-
extern char *pgstat_stat_directory;
646-
extern char *pgstat_stat_tmpname;
647-
extern char *pgstat_stat_filename;
648-
649645

650646
/*
651647
* Variables in pgstat_bgwriter.c

‎src/test/perl/PostgreSQL/Test/Cluster.pm‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,6 @@ sub init
480480
print $conf PostgreSQL::Test::Utils::slurp_file($ENV{TEMP_CONFIG})
481481
if defined $ENV{TEMP_CONFIG};
482482

483-
# XXX Neutralize any stats_temp_directory in TEMP_CONFIG. Nodes running
484-
# concurrently must not share a stats_temp_directory.
485-
print $conf "stats_temp_directory = 'pg_stat_tmp'\n";
486-
487483
if ($params{allows_streaming})
488484
{
489485
if ($params{allows_streaming} eq "logical")

0 commit comments

Comments
 (0)