public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
Subject: [PATCH v56 5/6] Remove the GUC stats_temp_directory
Date: Tue, 29 Sep 2020 22:59:33 +0900
The new stats collection system doesn't need temporary directory, so
just remove it. pg_stat_statements modified to use pg_stat directory
to store its temporary files. As the result basebackup copies the
pg_stat_statments' temporary file if exists.
---
.../pg_stat_statements/pg_stat_statements.c | 13 +++---
doc/src/sgml/backup.sgml | 2 -
doc/src/sgml/config.sgml | 19 --------
doc/src/sgml/storage.sgml | 6 ---
src/backend/postmaster/pgstat.c | 10 -----
src/backend/replication/basebackup.c | 36 ----------------
src/backend/utils/misc/guc.c | 43 -------------------
src/backend/utils/misc/postgresql.conf.sample | 1 -
src/bin/initdb/initdb.c | 1 -
src/bin/pg_rewind/filemap.c | 7 ---
src/include/pgstat.h | 3 --
src/test/perl/PostgresNode.pm | 4 --
12 files changed, 6 insertions(+), 139 deletions(-)
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 62cccbfa44..28279f97d5 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -89,14 +89,13 @@ 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. We don't keep it in the core system's
+ * pg_stats. pg_stat_statements has no way of avoiding race conditions even if
+ * the directory were specified by a GUC. Besides, we only expect modest,
+ * infrequent I/O for query strings, so placing the file on a faster filesystem
+ * is not compelling.
*/
-#define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat"
+#define PGSS_TEXT_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pgss_query_texts.stat"
/* Magic number identifying the stats file format */
static const uint32 PGSS_FILE_HEADER = 0x20201218;
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index c5557d5444..875769a57e 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1155,8 +1155,6 @@ SELECT pg_stop_backup();
<filename>pg_snapshots/</filename>, <filename>pg_stat_tmp/</filename>,
and <filename>pg_subtrans/</filename> (but not the directories themselves) can be
omitted from the backup as they will be initialized on postmaster startup.
- If <xref linkend="guc-stats-temp-directory"/> is set and is under the data
- directory then the contents of that directory can also be omitted.
</para>
<para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 53d7dfda93..9a749447ec 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7509,25 +7509,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</listitem>
</varlistentry>
- <varlistentry id="guc-stats-temp-directory" xreflabel="stats_temp_directory">
- <term><varname>stats_temp_directory</varname> (<type>string</type>)
- <indexterm>
- <primary><varname>stats_temp_directory</varname> configuration parameter</primary>
- </indexterm>
- </term>
- <listitem>
- <para>
- Sets the directory to store temporary statistics data in. This can be
- a path relative to the data directory or an absolute path. The default
- is <filename>pg_stat_tmp</filename>. Pointing this at a RAM-based
- file system will decrease physical I/O requirements and can lead to
- improved performance.
- This parameter can only be set in the <filename>postgresql.conf</filename>
- file or on the server command line.
- </para>
- </listitem>
- </varlistentry>
-
</variablelist>
</sect2>
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 3234adb639..6bac5e075e 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -120,12 +120,6 @@ Item
subsystem</entry>
</row>
-<row>
- <entry><filename>pg_stat_tmp</filename></entry>
- <entry>Subdirectory containing temporary files for the statistics
- subsystem</entry>
-</row>
-
<row>
<entry><filename>pg_subtrans</filename></entry>
<entry>Subdirectory containing subtransaction status data</entry>
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 575ecdd502..c09fa026b9 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -99,16 +99,6 @@ bool pgstat_track_counts = false;
int pgstat_track_functions = TRACK_FUNC_OFF;
int pgstat_track_activity_query_size = 1024;
-/* ----------
- * 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;
-
/*
* WAL usage counters saved from pgWALUsage at the previous call to
* pgstat_report_wal(). This is used to calculate how much WAL usage
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index fa7bdbcefa..f0d75f55bd 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -87,9 +87,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;
-
/*
* Size of each block sent into the tar stream for larger files.
*/
@@ -152,13 +149,6 @@ 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.
- */
- PG_STAT_TMP_DIR,
-
/*
* It is generally not useful to backup the contents of this directory
* even if the intention is to restore to another primary. See backup.sgml
@@ -261,7 +251,6 @@ perform_base_backup(basebackup_options *opt)
StringInfo labelfile;
StringInfo tblspc_map_file;
backup_manifest_info manifest;
- int datadirpathlen;
List *tablespaces = NIL;
backup_total = 0;
@@ -284,8 +273,6 @@ perform_base_backup(basebackup_options *opt)
Assert(CurrentResourceOwner == NULL);
CurrentResourceOwner = ResourceOwnerCreate(NULL, "base backup");
- datadirpathlen = strlen(DataDir);
-
backup_started_in_recovery = RecoveryInProgress();
labelfile = makeStringInfo();
@@ -314,18 +301,6 @@ perform_base_backup(basebackup_options *opt)
tablespaceinfo *ti;
int tblspc_streamed = 0;
- /*
- * 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;
@@ -1377,17 +1352,6 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
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);
- size += _tarWriteDir(pathbuf, basepathlen, &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 b22fa6b86e..99bca927ce 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -203,7 +203,6 @@ static bool check_autovacuum_work_mem(int *newval, void **extra, GucSource sourc
static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source);
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 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);
@@ -560,8 +559,6 @@ char *HbaFileName;
char *IdentFileName;
char *external_pid_file;
-char *pgstat_temp_directory;
-
char *application_name;
int tcp_keepalives_idle;
@@ -4365,17 +4362,6 @@ static struct config_string ConfigureNamesString[] =
NULL, NULL, NULL
},
- {
- {"stats_temp_directory", PGC_SIGHUP, STATS_ACTIVITY,
- 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."),
@@ -11781,35 +11767,6 @@ check_huge_page_size(int *newval, void **extra, GucSource source)
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 b0e73024a1..92aace4208 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -589,7 +589,6 @@
#track_wal_io_timing = off
#track_functions = none # none, pl, all
#track_activity_query_size = 1024 # (change requires restart)
-#stats_temp_directory = 'pg_stat_tmp'
# - Monitoring -
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 3c1cf78b4f..07a00b8d0d 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -219,7 +219,6 @@ static const char *const subdirs[] = {
"pg_replslot",
"pg_tblspc",
"pg_stat",
- "pg_stat_tmp",
"pg_xact",
"pg_logical",
"pg_logical/snapshots",
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 2618b4c957..ab5cb51de7 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -87,13 +87,6 @@ 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.
- */
- "pg_stat_tmp", /* defined as PG_STAT_TMP_DIR */
-
/*
* It is generally not useful to backup the contents of this directory
* even if the intention is to restore to another primary. See backup.sgml
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index f4177eb284..e1c54e73f2 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -33,9 +33,6 @@
#define PGSTAT_STAT_PERMANENT_FILENAME "pg_stat/saved_stats"
#define PGSTAT_STAT_PERMANENT_TMPFILE "pg_stat/saved_stats.tmp"
-/* Default directory to store temporary statistics data in */
-#define PG_STAT_TMP_DIR "pg_stat_tmp"
-
/* Values for track_functions GUC variable --- order is significant! */
typedef enum TrackFunctionsLevel
{
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 9667f7667e..dd41a43b4e 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -455,10 +455,6 @@ sub init
print $conf TestLib::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.27.0
----Next_Part(Tue_Mar_16_10_27_55_2021_500)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v56-0006-Exclude-pg_stat-directory-from-base-backup.patch"
view thread (232+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v56 5/6] Remove the GUC stats_temp_directory
In-Reply-To: <no-message-id-1870612@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox