On 2020/06/29 18:56, Fujii Masao wrote:


On 2020/06/29 18:53, Julien Rouhaud wrote:
On Mon, Jun 29, 2020 at 11:38 AM Fujii Masao
<masao.fu...@oss.nttdata.com> wrote:

Your benchmark result seems to suggest that the cause of the problem is
the contention of per-query spinlock in pgss_store(). Right?
This lock contention is likely to happen when multiple sessions run
the same queries.

One idea to reduce that lock contention is to separate per-query spinlock
into two; one is for planning, and the other is for execution. pgss_store()
determines which lock to use based on the given "kind" argument.
To make this idea work, also every pgss counters like shared_blks_hit
need to be separated into two, i.e., for planning and execution.

This can probably remove some overhead, but won't it eventually hit
the same issue when multiple connections try to plan the same query,
given the number of different queries and very low execution runtime?

Yes. But maybe we can expect that the idea would improve
the performance to the near same level as v12?

A POC patch should be easy to do and see how much it solves this
problem.  However I'm not able to reproduce the issue, and IMHO unless
we specifically want to be able to distinguish planner-time counters
from execution-time counters, I'd prefer to disable track_planning by
default than going this way, so that users with a sane usage won't
have to suffer from a memory increase.

Agreed. +1 to change that default to off.

Attached patch does this.

I also add the following into the description about each *_plan_time column
in the docs. IMO this is helpful for users when they see that those columns
report zero by default and try to understand why.

(if <varname>pg_stat_statements.track_planning</varname> is enabled, otherwise 
zero)

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out 
b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index f615f8c2bf..c3f013860a 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -3,6 +3,7 @@ CREATE EXTENSION pg_stat_statements;
 -- simple and compound statements
 --
 SET pg_stat_statements.track_utility = FALSE;
+SET pg_stat_statements.track_planning = TRUE;
 SELECT pg_stat_statements_reset();
  pg_stat_statements_reset 
 --------------------------
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c 
b/contrib/pg_stat_statements/pg_stat_statements.c
index cef8bb5a49..65ac301b99 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -442,7 +442,7 @@ _PG_init(void)
                                                         "Selects whether 
planning duration is tracked by pg_stat_statements.",
                                                         NULL,
                                                         &pgss_track_planning,
-                                                        true,
+                                                        false,
                                                         PGC_SUSET,
                                                         0,
                                                         NULL,
diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql 
b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
index 75c10554a8..6ed8e38028 100644
--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
@@ -4,6 +4,7 @@ CREATE EXTENSION pg_stat_statements;
 -- simple and compound statements
 --
 SET pg_stat_statements.track_utility = FALSE;
+SET pg_stat_statements.track_planning = TRUE;
 SELECT pg_stat_statements_reset();
 
 SELECT 1 AS "int";
diff --git a/doc/src/sgml/pgstatstatements.sgml 
b/doc/src/sgml/pgstatstatements.sgml
index a13e28a84c..430d8bf07c 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -101,6 +101,8 @@
       </para>
       <para>
        Number of times the statement was planned
+       (if <varname>pg_stat_statements.track_planning</varname> is enabled,
+       otherwise zero)
       </para></entry>
      </row>
 
@@ -110,6 +112,8 @@
       </para>
       <para>
        Total time spent planning the statement, in milliseconds
+       (if <varname>pg_stat_statements.track_planning</varname> is enabled,
+       otherwise zero)
       </para></entry>
      </row>
 
@@ -119,6 +123,8 @@
       </para>
       <para>
        Minimum time spent planning the statement, in milliseconds
+       (if <varname>pg_stat_statements.track_planning</varname> is enabled,
+       otherwise zero)
       </para></entry>
      </row>
 
@@ -128,6 +134,8 @@
       </para>
       <para>
        Maximum time spent planning the statement, in milliseconds
+       (if <varname>pg_stat_statements.track_planning</varname> is enabled,
+       otherwise zero)
       </para></entry>
      </row>
 
@@ -137,6 +145,8 @@
       </para>
       <para>
        Mean time spent planning the statement, in milliseconds
+       (if <varname>pg_stat_statements.track_planning</varname> is enabled,
+       otherwise zero)
       </para></entry>
      </row>
 
@@ -145,7 +155,10 @@
        <structfield>stddev_plan_time</structfield> <type>double 
precision</type>
       </para>
       <para>
-       Population standard deviation of time spent planning the statement, in 
milliseconds
+       Population standard deviation of time spent planning the statement,
+       in milliseconds
+       (if <varname>pg_stat_statements.track_planning</varname> is enabled,
+       otherwise zero)
       </para></entry>
      </row>
 
@@ -594,7 +607,7 @@
      <para>
       <varname>pg_stat_statements.track_planning</varname> controls whether
       planning operations and duration are tracked by the module.
-      The default value is <literal>on</literal>.
+      The default value is <literal>off</literal>.
       Only superusers can change this setting.
      </para>
     </listitem>

Reply via email to