public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v46 6/7] Remove the GUC stats_temp_directory
3+ messages / 3 participants
[nested] [flat]
* [PATCH v46 6/7] Remove the GUC stats_temp_directory
@ 2020-09-29 13:59 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Kyotaro Horiguchi @ 2020-09-29 13:59 UTC (permalink / raw)
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 72a117fc19..0a98b2f2c0 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 3c8aaed0b6..7557a375f0 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1146,8 +1146,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 b0c25c9c5c..084bc57779 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7440,25 +7440,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 ecf9d9adcc..73b44a2652 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_send_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 d21801cf90..d2c3064678 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 85299e2138..16e430fb28 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;
@@ -4355,17 +4352,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."),
@@ -11773,35 +11759,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 4f5b6bdb12..20c24a9d78 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -587,7 +587,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/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index c854221a30..0f42e78d19 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -217,7 +217,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 0472b728bf..d7c50eb4f9 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(Thu_Jan_14_15_14_25_2021_903)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v46-0007-Exclude-pg_stat-directory-from-base-backup.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Report planning memory in EXPLAIN ANALYZE
@ 2023-12-01 02:57 Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Andrei Lepikhov @ 2023-12-01 02:57 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; Ashutosh Bapat <[email protected]>; +Cc: Andy Fan <[email protected]>; jian he <[email protected]>; David Rowley <[email protected]>; PostgreSQL Hackers <[email protected]>
On 30/11/2023 18:40, Alvaro Herrera wrote:
> Well, SUMMARY is enabled by default with ANALYZE, and I'd rather not
> have planner memory consumption displayed by default with all EXPLAIN
> ANALYZEs. So yeah, I still think this deserves its own option.
>
> But let's hear others' opinions on this point. I'm only one vote here.
I agree; it should be disabled by default. The fact that memory
consumption outputs with byte precision (very uncomfortable for
perception) is a sign that the primary audience is developers and for
debugging purposes.
--
regards,
Andrei Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Report planning memory in EXPLAIN ANALYZE
@ 2023-12-04 07:24 Ashutosh Bapat <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Ashutosh Bapat @ 2023-12-04 07:24 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: Andy Fan <[email protected]>; jian he <[email protected]>; David Rowley <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Dec 1, 2023 at 8:27 AM Andrei Lepikhov
<[email protected]> wrote:
>
> On 30/11/2023 18:40, Alvaro Herrera wrote:
> > Well, SUMMARY is enabled by default with ANALYZE, and I'd rather not
> > have planner memory consumption displayed by default with all EXPLAIN
> > ANALYZEs. So yeah, I still think this deserves its own option.
> >
> > But let's hear others' opinions on this point. I'm only one vote here.
>
> I agree; it should be disabled by default. The fact that memory
> consumption outputs with byte precision (very uncomfortable for
> perception) is a sign that the primary audience is developers and for
> debugging purposes.
That's 2 vs 1. Here's patch with MEMORY option added. Replying to
Alvaro's earlier relevant comments.
> If EXPLAIN (MEMORY) is added, then probably auto_explain needs a
> corresponding flag, and doc updates.
auto_explain does not implement planner_hook and hence can not gather
information about planner's memory usage. So no new flag and doc
update to auto_explain. I have added a comment auto_explain.c but
probably even that's not needed. We are considering this option only
for developers and auto_explain is largely for users. So I didn't feel
like adding an implementation of planner_hook in auto_explain and use
MEMORY option. If we feel that planner's memory usage report is useful
in auto_explain, it should easy to do that in future.
> The EXPLAIN docs (explain.sgml) need an update to mention the new flag
> and the new output, too.
Done.
0001 is as is except explain.out and explain.sgml changes reverted.
0002 following changes. It's a separate patch for ease of review.
a. implements MEMORY option, adds tests in explain.sql and also
updates explain.sgml.
b. show_planning_memory renamed to show_planner_memory to be in sync
with the output
c. small indentation change by pgindent.
--
Best Wishes,
Ashutosh Bapat
Attachments:
[text/x-patch] 0002-EXPLAIN-reports-memory-consumed-for-plannin-20231204.patch (10.7K, ../../CAExHW5stHDXdRi1_pvNwq5R=qEseBeUxL6_v2ZRT3Pns2K6Jsw@mail.gmail.com/2-0002-EXPLAIN-reports-memory-consumed-for-plannin-20231204.patch)
download | inline diff:
From fdd3919559cd4307536c358095fe3254bad7ab0d Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Mon, 4 Dec 2023 12:12:03 +0530
Subject: [PATCH 2/2] EXPLAIN reports memory consumed for planning a query
The memory consumed is reported as "Planner Memory" property in EXPLAIN output
when any of the option MEMORY is specified.
auto_explain does not use this option since it does not planner_hook and has no
way to gather information about planner's memory usage.
Ashutosh Bapat
---
contrib/auto_explain/auto_explain.c | 5 ++
doc/src/sgml/ref/explain.sgml | 24 ++++++++++
src/backend/commands/explain.c | 14 +++---
src/backend/commands/prepare.c | 2 +-
src/include/commands/explain.h | 1 +
src/test/regress/expected/explain.out | 68 +++++++++++++++++++++++++++
src/test/regress/sql/explain.sql | 6 +++
7 files changed, 113 insertions(+), 7 deletions(-)
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index c3ac27ae99..8a77e12605 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -390,6 +390,11 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
{
ExplainState *es = NewExplainState();
+ /*
+ * auto_explain doesn't implement planner_hook and hence can not
+ * gather information about planner's memory usage. Hence we leave
+ * es->memory = false.
+ */
es->analyze = (queryDesc->instrument_options && auto_explain_log_analyze);
es->verbose = auto_explain_log_verbose;
es->buffers = (es->analyze && auto_explain_log_buffers);
diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml
index 5ba6486da1..a0a0472e4e 100644
--- a/doc/src/sgml/ref/explain.sgml
+++ b/doc/src/sgml/ref/explain.sgml
@@ -44,6 +44,7 @@ EXPLAIN [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] <rep
WAL [ <replaceable class="parameter">boolean</replaceable> ]
TIMING [ <replaceable class="parameter">boolean</replaceable> ]
SUMMARY [ <replaceable class="parameter">boolean</replaceable> ]
+ MEMORY [ <replaceable class="parameter">boolean</replaceable> ]
FORMAT { TEXT | XML | JSON | YAML }
</synopsis>
</refsynopsisdiv>
@@ -250,6 +251,15 @@ ROLLBACK;
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>MEMORY</literal></term>
+ <listitem>
+ <para>
+ Include information on planner's memory consumption. Specially, include the total memory allocated by the planner and net memory that remains used at the end of the planning. It defaults to <literal>FALSE</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>FORMAT</literal></term>
<listitem>
@@ -510,6 +520,20 @@ EXPLAIN (GENERIC_PLAN)
SELECT sum(bar) FROM test
WHERE id > $1::integer AND id < $2::integer
GROUP BY foo;
+</programlisting>
+ </para>
+
+ <para>
+ Here's an example using MEMORY option.
+
+<programlisting>
+EXPLAIN (MEMORY) SELECT * FROM foo;
+
+ QUERY PLAN
+---------------------------------------------------------
+ Seq Scan on foo (cost=0.00..155.00 rows=10000 width=4)
+ Planner Memory: used=22688 bytes allocated=32768 bytes
+(2 rows)
</programlisting>
</para>
</refsect1>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 8c7f27b661..e94da4e7f8 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -122,8 +122,8 @@ static const char *explain_get_index_name(Oid indexId);
static void show_buffer_usage(ExplainState *es, const BufferUsage *usage,
bool planning);
static void show_wal_usage(ExplainState *es, const WalUsage *usage);
-static void show_planning_memory(ExplainState *es,
- const MemoryUsage *usage);
+static void show_planner_memory(ExplainState *es,
+ const MemoryUsage *usage);
static void ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir,
ExplainState *es);
static void ExplainScanTarget(Scan *plan, ExplainState *es);
@@ -204,6 +204,8 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt,
summary_set = true;
es->summary = defGetBoolean(opt);
}
+ else if (strcmp(opt->defname, "memory") == 0)
+ es->memory = defGetBoolean(opt);
else if (strcmp(opt->defname, "format") == 0)
{
char *p = defGetString(opt);
@@ -402,7 +404,7 @@ ExplainOneQuery(Query *query, int cursorOptions,
bufusage;
MemoryContextCounters mem_counts_start;
MemoryContextCounters mem_counts_end;
- MemoryUsage mem_usage;
+ MemoryUsage mem_usage;
MemoryContext saved_ctx;
/*
@@ -653,10 +655,10 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
ExplainPropertyFloat("Planning Time", "ms", 1000.0 * plantime, 3, es);
}
- if (es->summary && mem_usage)
+ if (es->memory && mem_usage)
{
ExplainOpenGroup("Planner Memory", "Planner Memory", true, es);
- show_planning_memory(es, mem_usage);
+ show_planner_memory(es, mem_usage);
ExplainCloseGroup("Planner Memory", "Planner Memory", true, es);
}
@@ -3800,7 +3802,7 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
* Show planner's memory usage details.
*/
static void
-show_planning_memory(ExplainState *es, const MemoryUsage *usage)
+show_planner_memory(ExplainState *es, const MemoryUsage *usage)
{
if (es->format == EXPLAIN_FORMAT_TEXT)
{
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 3d3d0ae6a3..629155fca4 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -585,7 +585,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
bufusage;
MemoryContextCounters mem_counts_start;
MemoryContextCounters mem_counts_end;
- MemoryUsage mem_usage;
+ MemoryUsage mem_usage;
MemoryContext planner_ctx;
MemoryContext saved_ctx;
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 6947cbae8b..f2efc7741f 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -47,6 +47,7 @@ typedef struct ExplainState
bool summary; /* print total planning and execution timing */
bool settings; /* print modified settings */
bool generic; /* generate a generic plan */
+ bool memory; /* print planner's memory usage information */
ExplainFormat format; /* output format */
/* state for output formatting --- not reset for each new plan tree */
int indent; /* current indentation level */
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 809655e16e..86bfdfd29e 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -326,6 +326,74 @@ select explain_filter('explain (generic_plan) select unique1 from tenk1 where th
select explain_filter('explain (analyze, generic_plan) select unique1 from tenk1 where thousand = $1');
ERROR: EXPLAIN options ANALYZE and GENERIC_PLAN cannot be used together
CONTEXT: PL/pgSQL function explain_filter(text) line 5 at FOR over EXECUTE statement
+-- MEMORY option
+select explain_filter('explain (memory) select * from int8_tbl i8');
+ explain_filter
+---------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N)
+ Planner Memory: used=N bytes allocated=N bytes
+(2 rows)
+
+select explain_filter('explain (memory, analyze) select * from int8_tbl i8');
+ explain_filter
+-----------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
+ Planning Time: N.N ms
+ Planner Memory: used=N bytes allocated=N bytes
+ Execution Time: N.N ms
+(4 rows)
+
+select explain_filter('explain (memory, summary, format yaml) select * from int8_tbl i8');
+ explain_filter
+-------------------------------
+ - Plan: +
+ Node Type: "Seq Scan" +
+ Parallel Aware: false +
+ Async Capable: false +
+ Relation Name: "int8_tbl"+
+ Alias: "i8" +
+ Startup Cost: N.N +
+ Total Cost: N.N +
+ Plan Rows: N +
+ Plan Width: N +
+ Planning Time: N.N +
+ Planner Memory: +
+ Used: N +
+ Allocated: N
+(1 row)
+
+select explain_filter('explain (memory, analyze, format json) select * from int8_tbl i8');
+ explain_filter
+------------------------------------
+ [ +
+ { +
+ "Plan": { +
+ "Node Type": "Seq Scan", +
+ "Parallel Aware": false, +
+ "Async Capable": false, +
+ "Relation Name": "int8_tbl",+
+ "Alias": "i8", +
+ "Startup Cost": N.N, +
+ "Total Cost": N.N, +
+ "Plan Rows": N, +
+ "Plan Width": N, +
+ "Actual Startup Time": N.N, +
+ "Actual Total Time": N.N, +
+ "Actual Rows": N, +
+ "Actual Loops": N +
+ }, +
+ "Planning Time": N.N, +
+ "Planner Memory": { +
+ "Used": N, +
+ "Allocated": N +
+ }, +
+ "Triggers": [ +
+ ], +
+ "Execution Time": N.N +
+ } +
+ ]
+(1 row)
+
-- Test EXPLAIN (GENERIC_PLAN) with partition pruning
-- partitions should be pruned at plan time, based on constants,
-- but there should be no pruning based on parameter placeholders
diff --git a/src/test/regress/sql/explain.sql b/src/test/regress/sql/explain.sql
index b6b7beab27..f719e92f84 100644
--- a/src/test/regress/sql/explain.sql
+++ b/src/test/regress/sql/explain.sql
@@ -94,6 +94,12 @@ select explain_filter('explain (generic_plan) select unique1 from tenk1 where th
-- should fail
select explain_filter('explain (analyze, generic_plan) select unique1 from tenk1 where thousand = $1');
+-- MEMORY option
+select explain_filter('explain (memory) select * from int8_tbl i8');
+select explain_filter('explain (memory, analyze) select * from int8_tbl i8');
+select explain_filter('explain (memory, summary, format yaml) select * from int8_tbl i8');
+select explain_filter('explain (memory, analyze, format json) select * from int8_tbl i8');
+
-- Test EXPLAIN (GENERIC_PLAN) with partition pruning
-- partitions should be pruned at plan time, based on constants,
-- but there should be no pruning based on parameter placeholders
--
2.25.1
[text/x-patch] 0001-EXPLAIN-reports-memory-consumed-for-plannin-20231204.patch (11.8K, ../../CAExHW5stHDXdRi1_pvNwq5R=qEseBeUxL6_v2ZRT3Pns2K6Jsw@mail.gmail.com/3-0001-EXPLAIN-reports-memory-consumed-for-plannin-20231204.patch)
download | inline diff:
From cd4c580926179fdc469f0e1d1c393985203dae6b Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Wed, 12 Jul 2023 14:34:14 +0530
Subject: [PATCH 1/2] EXPLAIN reports memory consumed for planning a query
The memory consumed is reported as "Planner Memory" property in EXPLAIN
output when any of the options ANALYZE or SUMMARY is specified.
A separate memory context is allocated for measuring memory consumption
to eliminate any effect of previous activity on the calculation. The
memory context statistics is noted before and after calling
pg_plan_query() from ExplainOneQuery(). The difference in the two
statistics is used to calculate the memory consumption.
We report two values "used" and "allocated". The difference between
memory statistics counters totalspace and freespace gives the memory
that remains palloc'ed at the end of planning. It is reported as memory
"used". This does not account for chunks of memory that were palloc'ed
but subsequently pfree'ed. But such memory remains allocated in the
memory context is given by the totalspace counter. The value of this
counter is reported as memory "allocated".
Ashutosh Bapat, reviewed by David Rowley and Alvaro Herrera
---
src/backend/commands/explain.c | 55 ++++++++++++++++++++++++++++++--
src/backend/commands/prepare.c | 24 +++++++++++++-
src/backend/utils/mmgr/mcxt.c | 38 ++++++++++++++++++++++
src/include/commands/explain.h | 3 +-
src/include/nodes/memnodes.h | 10 ++++++
src/include/utils/memutils.h | 5 +++
src/tools/pgindent/typedefs.list | 1 +
7 files changed, 132 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index f1d71bc54e..8c7f27b661 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -122,6 +122,8 @@ static const char *explain_get_index_name(Oid indexId);
static void show_buffer_usage(ExplainState *es, const BufferUsage *usage,
bool planning);
static void show_wal_usage(ExplainState *es, const WalUsage *usage);
+static void show_planning_memory(ExplainState *es,
+ const MemoryUsage *usage);
static void ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir,
ExplainState *es);
static void ExplainScanTarget(Scan *plan, ExplainState *es);
@@ -393,13 +395,30 @@ ExplainOneQuery(Query *query, int cursorOptions,
else
{
PlannedStmt *plan;
+ MemoryContext planner_ctx;
instr_time planstart,
planduration;
BufferUsage bufusage_start,
bufusage;
+ MemoryContextCounters mem_counts_start;
+ MemoryContextCounters mem_counts_end;
+ MemoryUsage mem_usage;
+ MemoryContext saved_ctx;
+ /*
+ * Create a new memory context to accurately measure memory malloc'ed
+ * by the planner. For further accuracy we should use the same type of
+ * memory context as the planner would use. That's usually AllocSet
+ * but ensure that.
+ */
+ Assert(IsA(CurrentMemoryContext, AllocSetContext));
+ planner_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "explain analyze planner context",
+ ALLOCSET_DEFAULT_SIZES);
if (es->buffers)
bufusage_start = pgBufferUsage;
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_start);
+ saved_ctx = MemoryContextSwitchTo(planner_ctx);
INSTR_TIME_SET_CURRENT(planstart);
/* plan the query */
@@ -407,6 +426,9 @@ ExplainOneQuery(Query *query, int cursorOptions,
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ MemoryContextSwitchTo(saved_ctx);
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_end);
+ MemoryContextSizeDifference(&mem_usage, &mem_counts_start, &mem_counts_end);
/* calc differences of buffer counters. */
if (es->buffers)
@@ -417,7 +439,8 @@ ExplainOneQuery(Query *query, int cursorOptions,
/* run it (if needed) and produce output */
ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
- &planduration, (es->buffers ? &bufusage : NULL));
+ &planduration, (es->buffers ? &bufusage : NULL),
+ &mem_usage);
}
}
@@ -527,7 +550,7 @@ void
ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
QueryEnvironment *queryEnv, const instr_time *planduration,
- const BufferUsage *bufusage)
+ const BufferUsage *bufusage, const MemoryUsage *mem_usage)
{
DestReceiver *dest;
QueryDesc *queryDesc;
@@ -630,6 +653,13 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
ExplainPropertyFloat("Planning Time", "ms", 1000.0 * plantime, 3, es);
}
+ if (es->summary && mem_usage)
+ {
+ ExplainOpenGroup("Planner Memory", "Planner Memory", true, es);
+ show_planning_memory(es, mem_usage);
+ ExplainCloseGroup("Planner Memory", "Planner Memory", true, es);
+ }
+
/* Print info about runtime of triggers */
if (es->analyze)
ExplainPrintTriggers(es, queryDesc);
@@ -3766,6 +3796,27 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
}
}
+/*
+ * Show planner's memory usage details.
+ */
+static void
+show_planning_memory(ExplainState *es, const MemoryUsage *usage)
+{
+ if (es->format == EXPLAIN_FORMAT_TEXT)
+ {
+ appendStringInfo(es->str,
+ "Planner Memory: used=%zu bytes allocated=%zu bytes",
+ usage->bytes_used, usage->bytes_allocated);
+ appendStringInfoChar(es->str, '\n');
+ }
+ else
+ {
+ ExplainPropertyInteger("Used", "bytes", usage->bytes_used, es);
+ ExplainPropertyInteger("Allocated", "bytes", usage->bytes_allocated, es);
+ }
+}
+
+
/*
* Add some additional details about an IndexScan or IndexOnlyScan
*/
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 18f70319fc..3d3d0ae6a3 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -583,9 +583,27 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
instr_time planduration;
BufferUsage bufusage_start,
bufusage;
+ MemoryContextCounters mem_counts_start;
+ MemoryContextCounters mem_counts_end;
+ MemoryUsage mem_usage;
+ MemoryContext planner_ctx;
+ MemoryContext saved_ctx;
+
+ /*
+ * Create a new memory context to accurately measure memory malloc'ed by
+ * the planner. For further accuracy we should use the same type of memory
+ * context as the planner would use. That's usually AllocSet but ensure
+ * that.
+ */
+ Assert(IsA(CurrentMemoryContext, AllocSetContext));
+ planner_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "explain analyze planner context",
+ ALLOCSET_DEFAULT_SIZES);
if (es->buffers)
bufusage_start = pgBufferUsage;
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_start);
+ saved_ctx = MemoryContextSwitchTo(planner_ctx);
INSTR_TIME_SET_CURRENT(planstart);
/* Look it up in the hash table */
@@ -623,6 +641,9 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ MemoryContextSwitchTo(saved_ctx);
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_end);
+ MemoryContextSizeDifference(&mem_usage, &mem_counts_start, &mem_counts_end);
/* calc differences of buffer counters. */
if (es->buffers)
@@ -640,7 +661,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
if (pstmt->commandType != CMD_UTILITY)
ExplainOnePlan(pstmt, into, es, query_string, paramLI, queryEnv,
- &planduration, (es->buffers ? &bufusage : NULL));
+ &planduration, (es->buffers ? &bufusage : NULL),
+ &mem_usage);
else
ExplainOneUtility(pstmt->utilityStmt, into, es, query_string,
paramLI, queryEnv);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6..28e5f36405 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -687,6 +687,32 @@ MemoryContextMemAllocated(MemoryContext context, bool recurse)
return total;
}
+/*
+ * Compute memory usage from the start and end memory counts.
+ */
+void
+MemoryContextSizeDifference(MemoryUsage *mem_usage,
+ MemoryContextCounters *start,
+ MemoryContextCounters *end)
+{
+ /*
+ * We compute the memory "used" as the difference, between end situation
+ * and start situation, of the memory that's allocated according to the
+ * counters, excluding memory in freelists.
+ */
+ mem_usage->bytes_used =
+ (end->totalspace - end->freespace) -
+ (start->totalspace - start->freespace);
+
+ /*
+ * The net memory used is from total memory allocated and not necessarily
+ * the net memory allocated between the two given samples. Hence do not
+ * compute the difference between allocated memory reported in the two
+ * given samples.
+ */
+ mem_usage->bytes_allocated = end->totalspace;
+}
+
/*
* MemoryContextStats
* Print statistics about the named context and all its descendants.
@@ -747,6 +773,18 @@ MemoryContextStatsDetail(MemoryContext context, int max_children,
grand_totals.totalspace - grand_totals.freespace)));
}
+/*
+ * Return the memory used in the given context and its children.
+ */
+extern void
+MemoryContextMemConsumed(MemoryContext context,
+ MemoryContextCounters *consumed)
+{
+ memset(consumed, 0, sizeof(*consumed));
+
+ MemoryContextStatsInternal(context, 0, false, 0, consumed, false);
+}
+
/*
* MemoryContextStatsInternal
* One recursion level for MemoryContextStats
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index f9525fb572..6947cbae8b 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -92,7 +92,8 @@ extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv,
const instr_time *planduration,
- const BufferUsage *bufusage);
+ const BufferUsage *bufusage,
+ const MemoryUsage *mem_usage);
extern void ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc);
extern void ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc);
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index ff6453bb7a..577d39cef4 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -34,6 +34,16 @@ typedef struct MemoryContextCounters
Size freespace; /* The unused portion of totalspace */
} MemoryContextCounters;
+/*
+ * MemoryUsage
+ * For concise reporting of memory consumption
+ */
+typedef struct MemoryUsage
+{
+ Size bytes_used;
+ Size bytes_allocated;
+} MemoryUsage;
+
/*
* MemoryContext
* A logical context in which memory allocations occur.
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index d14e8546a6..24dc0e996e 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -84,6 +84,11 @@ extern Size GetMemoryChunkSpace(void *pointer);
extern MemoryContext MemoryContextGetParent(MemoryContext context);
extern bool MemoryContextIsEmpty(MemoryContext context);
extern Size MemoryContextMemAllocated(MemoryContext context, bool recurse);
+extern void MemoryContextMemConsumed(MemoryContext context,
+ MemoryContextCounters *consumed);
+extern void MemoryContextSizeDifference(MemoryUsage *mem_usage,
+ MemoryContextCounters *start,
+ MemoryContextCounters *end);
extern void MemoryContextStats(MemoryContext context);
extern void MemoryContextStatsDetail(MemoryContext context, int max_children,
bool print_to_stderr);
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d659adbfd6..0691135b67 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1554,6 +1554,7 @@ MemoryContextData
MemoryContextMethodID
MemoryContextMethods
MemoryStatsPrintFunc
+MemoryUsage
MergeAction
MergeActionState
MergeAppend
--
2.25.1
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-12-04 07:24 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 13:59 [PATCH v46 6/7] Remove the GUC stats_temp_directory Kyotaro Horiguchi <[email protected]>
2023-12-01 02:57 Re: Report planning memory in EXPLAIN ANALYZE Andrei Lepikhov <[email protected]>
2023-12-04 07:24 ` Re: Report planning memory in EXPLAIN ANALYZE Ashutosh Bapat <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox