public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
Subject: [PATCH 5/5] Remove the GUC stats_temp_directory
Date: Tue, 27 Nov 2018 14:42:12 +0900
The guc used to specifie the directory to store temporary statistics
files. It is no longer needed by the stats collector but still used by
the programs in bin and contirb, and maybe other extensions. Thus this
patch removes the GUC but some backing variables and macro definitions
are left alone for backward comptibility.
---
doc/src/sgml/backup.sgml | 2 --
doc/src/sgml/config.sgml | 19 -------------
doc/src/sgml/monitoring.sgml | 7 +----
doc/src/sgml/storage.sgml | 3 +-
src/backend/postmaster/pgstat.c | 13 ++++-----
src/backend/replication/basebackup.c | 13 ++-------
src/backend/utils/misc/guc.c | 41 ---------------------------
src/backend/utils/misc/postgresql.conf.sample | 1 -
src/include/pgstat.h | 5 +++-
src/test/perl/PostgresNode.pm | 4 ---
10 files changed, 14 insertions(+), 94 deletions(-)
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 9d4c000df0..bbae70221e 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1145,8 +1145,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 c91e3e1550..b85f6f421a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6827,25 +6827,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/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 990995c17b..2a2adaa0f7 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -195,12 +195,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<para>
The statistics collector transmits the collected information to other
- <productname>PostgreSQL</productname> processes through temporary files.
- These files are stored in the directory named by the
- <xref linkend="guc-stats-temp-directory"/> parameter,
- <filename>pg_stat_tmp</filename> by default.
- For better performance, <varname>stats_temp_directory</varname> can be
- pointed at a RAM-based file system, decreasing physical I/O requirements.
+ <productname>PostgreSQL</productname> processes through shared memory.
When the server shuts down cleanly, a permanent copy of the statistics
data is stored in the <filename>pg_stat</filename> subdirectory, so that
statistics can be retained across server restarts. When recovery is
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 1047c77a63..b47f8d084e 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -122,8 +122,7 @@ Item
<row>
<entry><filename>pg_stat_tmp</filename></entry>
- <entry>Subdirectory containing temporary files for the statistics
- subsystem</entry>
+ <entry>Subdirectory containing ephemeral files for extensions</entry>
</row>
<row>
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 514ea78a68..64e1374288 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -108,15 +108,12 @@ bool pgstat_track_counts = false;
int pgstat_track_functions = TRACK_FUNC_OFF;
int pgstat_track_activity_query_size = 1024;
-/* ----------
- * Built from GUC parameter
- * ----------
+/*
+ * This used to be a GUC variable and is no longer used in this file, but left
+ * alone just for backward comptibility for extensions, having the default
+ * value.
*/
-char *pgstat_stat_directory = NULL;
-
-/* No longer used, but will be removed with GUC */
-char *pgstat_stat_filename = NULL;
-char *pgstat_stat_tmpname = NULL;
+char *pgstat_stat_directory = PG_STAT_TMP_DIR;
#define StatsLock (&StatsShmem->StatsMainLock)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 57f17e1418..c854d7e193 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -230,11 +230,8 @@ perform_base_backup(basebackup_options *opt)
TimeLineID endtli;
StringInfo labelfile;
StringInfo tblspc_map_file = NULL;
- int datadirpathlen;
List *tablespaces = NIL;
- datadirpathlen = strlen(DataDir);
-
backup_started_in_recovery = RecoveryInProgress();
labelfile = makeStringInfo();
@@ -265,13 +262,9 @@ perform_base_backup(basebackup_options *opt)
* 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;
+
+ Assert(strchr(PG_STAT_TMP_DIR, '/') == NULL);
+ statrelpath = psprintf("./%s", PG_STAT_TMP_DIR);
/* Add a node for the base directory at the end */
ti = palloc0(sizeof(tablespaceinfo));
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fc463601ff..05af81ac47 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -194,7 +194,6 @@ static bool check_max_wal_senders(int *newval, void **extra, GucSource source);
static bool check_autovacuum_work_mem(int *newval, void **extra, GucSource source);
static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source);
static void assign_effective_io_concurrency(int newval, void *extra);
-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);
@@ -4085,17 +4084,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_MASTER,
gettext_noop("Number of synchronous standbys and list of names of potential synchronous ones."),
@@ -11368,35 +11356,6 @@ assign_effective_io_concurrency(int newval, void *extra)
#endif /* USE_PREFETCH */
}
-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 cfad86c02a..29b7ebca46 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -562,7 +562,6 @@
#track_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/include/pgstat.h b/src/include/pgstat.h
index c0bbf8a7d5..2c7c799ca9 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -31,7 +31,10 @@
#define PGSTAT_STAT_PERMANENT_FILENAME "pg_stat/global.stat"
#define PGSTAT_STAT_PERMANENT_TMPFILE "pg_stat/global.tmp"
-/* Default directory to store temporary statistics data in */
+/*
+ * This used to be the directory to store temporary statistics data in but is
+ * no longer used. Defined here for backward compatibility.
+ */
#define PG_STAT_TMP_DIR "pg_stat_tmp"
/* Values for track_functions GUC variable --- order is significant! */
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 6019f37f91..a107683c0d 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -451,10 +451,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.16.3
----Next_Part(Thu_Jul_11_16_40_59_2019_397)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="statbench.pl"
#! /usr/bin/perl
use strict;
use IPC::Open2;
use Time::HiRes qw( usleep ualarm gettimeofday tv_interval );
use Errno qw/ECHILD/;
my $totaldbs = 100;
my $totaltbls = 100;
my $ndbs = 50;
my $ntbls = 100;
my $loops = 20;
my $chiter = 20000;
my $chtrlen = 1000;
my $nprocs = 90;
my $trg_file = '/tmp/dbrun_pl.trg';
my $refresult_file = '/tmp/refresult.txt';
# title loops,ndbs,clnts,ntbls,childiter , xactlen,refprocs
# testrun("A1", $loops, 1, 1, 1, $chiter*10, $chtrlen, 0);
# testrun("A2", $loops, 1, 1, 1, $chiter*10, $chtrlen, 1);
# testrun("B1", $loops, 1, 90, 1, $chiter , $chtrlen, 0);
# testrun("B2", $loops, 1, 90, 1, $chiter , $chtrlen, 1);
# testrun("C1", $loops, 1, 90, 50, $chiter , $chtrlen, 0);
# testrun("C2", $loops, 1, 90, 50, $chiter , $chtrlen, 10);
# testrun("D1", $loops, 50, 90, 1, $chiter , $chtrlen, 0);
# testrun("D2", $loops, 50, 90, 1, $chiter , $chtrlen, 1);
# testrun("E1", $loops, 50, 90, 1, $chiter , $chtrlen, 10);
# testrun("F1", $loops , 1, 10, 1, $chiter , $chtrlen, 90);
testrun("G1", 10, 1, 400, 1000, 20000, 1000, 0);
testrun("G2", 10, 1, 400, 1000, 20000, 1000, 1);
exit;
sub testrun
{
my ($test_name, $loops, $ndbs, $nprocs, $ntbls,
$childiter, $childtrlen, $nreferers)
= @_;
my @results = ();
my @refresults = ();
# run each iteration
for (my $l = 0 ; $l < $loops ; $l++)
{
my %starttime = ();
my %endtime = ();
# This file is used for stopping free-running processes.
open(OUT, '>', $trg_file) || die "failed to open file:$!\n";
print OUT "$$\n";
close(OUT);
# It's very mysterious that one dummy subprocess makes things
# stable. Perl runs slowly if there's only one psql process
# without this...
{
my $pid = fork;
if ($pid == 0)
{
while (-f $trg_file)
{
usleep (500000);
}
exit;
}
}
# start referer processes, collecting how many times the query ran.
pipe(my $refresrd, my $refreswr);
for(my $i = 0 ; $i < $nreferers ; $i++)
{
my $pid = fork;
if ($pid < 0) { die "fork referer process failed : $!\n"; }
if ($pid == 0)
{
my $pid = open2(my $psqlrd, my $psqlwr, "psql postgres");
my $count = 0;
close($refresrd);
if ($pid < 0) { die "fork psql failed : $!\n"; }
while (-f $trg_file)
{
print $psqlwr "select * from pg_stat_user_tables;\n";
while (<$psqlrd>)
{
last if ($_ =~ /rows\)/);
}
$count++;
}
print $refreswr "$count\n";
exit;
}
}
close($refreswr);
# launch updator processes
for (my $i = 0 ; $i < $nprocs ; $i++)
{
#my $dbn = rand($ndbs); # equal dist.
my $dbn = $i % $totaldbs; # round robbin
my $dbname = sprintf("db%03d", $dbn);
my $pid = fork;
if ($pid < 0)
{
die "fork failed: $!\n";
}
elsif ($pid == 0)
{
my $pid = open2(my $rd, my $wr, "psql $dbname > /dev/null");
if ($pid < 0) { die "sub fork failed: $!\n"; }
my $ncmd = $childtrlen;
#print $wr "set log_min_duration_statement to 0;\n";
print $wr "begin;\n";
for (my $i = 0 ; $i < $childiter ; $i++)
{
#my $tbn = rand($ntbls); # equal dist.
my $tbn = $i % $totaltbls; # round robbin
printf $wr "select /* $dbname\[$i\]*/ count(*) from t%03d;\n", $tbn;
if (--$ncmd == 0)
{
print $wr "commit;begin;\n";
$ncmd = $childtrlen;
}
}
print $wr "commit;\n";
print $wr "\\q\n";
my $res = <$rd>;
exit;
}
(my $sec, my $usec) = &gettimeofday();
$starttime{$pid} = $sec * 1000 + $usec / 1000;
# print "start\[$pid\] = $starttime{$pid}\n";
}
# wait for updateors to finish
for (my $i = 0 ; $i < $nprocs ; $i++)
{
my $pid = wait();
if ($pid < 0) { if ($! != ECHILD) {die "???: $!\n"; }}
redo if (!defined $starttime{$pid});
(my $sec, my $usec) = &gettimeofday();
$endtime{$pid} = $sec * 1000 + $usec / 1000;
# printf "%d[%d]: %d - %d = %d ms\n", $i, $pid, $endtime{$pid}, $starttime{$pid}, $endtime{$pid} - $starttime{$pid};
}
my $sum = 0;
foreach my $pid (keys %starttime)
{
$sum += $endtime{$pid} - $starttime{$pid};
# printf "[%d]: %d ms (%d, %d)\n", $pid, $endtime{$pid} - $starttime{$pid}, $starttime{$pid}, $endtime{$pid};
}
push(@results, $sum / $nprocs);
# kill referers if any (and dummy process)
unlink($trg_file);
while (wait() == 0) {}
my $nrefs = 0;
my $refcount = 0;
while(<$refresrd>)
{
chomp;
$refcount += $_;
$nrefs++;
}
close($refresrd);
push (@refresults, $refcount / $nrefs) if ($nrefs > 0);
}
# calculate stdev
(my $updmean, my $updstdev) = stdev(@results);
(my $refmean, my $refstdev) = stdev(@refresults);
printf "$test_name (l:%d, d:%d, t:%d, i:%d, tr:%d): %.2f ms (stdev %.2f) / %d updprocs, %.2f refs (stdev %.2f) / %d refprocs\n", $loops, $ndbs, $ntbls, $childiter, $childtrlen, $updmean, $updstdev, $nprocs, $refmean, $refstdev, $nreferers;
}
sub stdev
{
my $sum = 0;
my $sqsum = 0;
my $count = $#_ + 1;
return (0, 0) if $count == 0;
foreach my $el (@_)
{
$sum += 1.0 * $el;
$sqsum += 1.0 * $el * $el;
}
my $mean = $sum / $count;
my $stdev = sqrt($sqsum / $count - $mean * $mean);
return ($mean, $stdev);
}
----Next_Part(Thu_Jul_11_16_40_59_2019_397)----
view thread (111+ 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 5/5] Remove the GUC stats_temp_directory
In-Reply-To: <no-message-id-1882033@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