pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
[PATCH] force_parallel_mode and GUC categories
18+ messages / 5 participants
[nested] [flat]

* [PATCH] force_parallel_mode and GUC categories
@ 2021-04-04 01:25 Justin Pryzby <pryzby@telsasoft.com>
  2021-04-08 21:38 ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  0 siblings, 2 replies; 18+ messages in thread

From: Justin Pryzby @ 2021-04-04 01:25 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

Forking this thread
https://www.postgresql.org/message-id/20210403154336.GG29125%40momjian.us

On Sat, Apr  3, 2021 at 08:38:18PM +0530, aditya desai wrote:
> > > >> Yes, force_parallel_mode is on. Should we set it off?

Bruce Momjian <bruce@momjian.us> writes:
> > > > Yes.  I bet someone set it without reading our docs:
...
> > > > We might need to clarify this sentence to be clearer it is _only_ for
> > > > testing.

On Sat, Apr 03, 2021 at 11:39:19AM -0400, Tom Lane wrote:
> > > I wonder why it is listed under planner options at all, and not under
> > > developer options.

On Sat, Apr  3, 2021 at 10:41:14AM -0500, Justin Pryzby wrote:
> > Because it's there to help DBAs catch errors in functions incorrectly marked as
> > parallel safe.

On Sat, Apr 03, 2021 at 11:43:36AM -0400, Bruce Momjian wrote:
> Uh, isn't that developer/debugging?

I understood "developer" to mean someone who's debugging postgres itself, not
(say) a function written using pl/pgsql.  Like backtrace_functions,
post_auth_delay, jit_profiling_support.

But I see that some "dev" options are more user-facing (for a sufficiently
advanced user):
ignore_checksum_failure, ignore_invalid_pages, zero_damaged_pages.

Also, I understood this to mean the "category" in pg_settings, but I guess
what's important here is the absense of the GUC in the sample/template config
file.  pg_settings.category and the sample headings it appears are intended to
be synchronized, but a few of them are out of sync.  See attached.

+1 to move this to "developer" options and remove it from the sample config:

# - Other Planner Options -
#force_parallel_mode = off

-- 
Justin

Attachments:

  [text/x-diff] 0001-track_activity_query_size-is-STATS_COLLECTOR-categor.patch (2.1K, ../../20210404012546.GK6592@telsasoft.com/2-0001-track_activity_query_size-is-STATS_COLLECTOR-categor.patch)
  download | inline diff:
From ce0c5b99650859bb13f204ea56221ca854a601e1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:06:37 -0500
Subject: [PATCH 1/4] track_activity_query_size is STATS_COLLECTOR category

Not Resource Usage / Memory, as since 995fb7420
---
 contrib/postgres_fdw/postgres_fdw.c | 1 +
 src/backend/utils/misc/guc.c        | 2 +-
 src/include/pg_config_manual.h      | 4 ++--
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 16c2979f2d..aff6e8c085 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -6797,6 +6797,7 @@ fetch_more_data_begin(AsyncRequest *areq)
 	char		sql[64];
 
 	Assert(!fsstate->conn_state->pendingAreq);
+	Assert(fsstate->conn);
 
 	/* Create the cursor synchronously. */
 	if (!fsstate->cursor_exists)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 584daffc8a..c315dd4bc1 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3435,7 +3435,7 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"track_activity_query_size", PGC_POSTMASTER, RESOURCES_MEM,
+		{"track_activity_query_size", PGC_POSTMASTER, STATS_COLLECTOR,
 			gettext_noop("Sets the size reserved for pg_stat_activity.query, in bytes."),
 			NULL,
 			GUC_UNIT_BYTE
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index e28c990382..fdb4fc1de2 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -288,7 +288,7 @@
  * You should normally use MEMORY_CONTEXT_CHECKING with USE_VALGRIND;
  * instrumentation of repalloc() is inferior without it.
  */
-/* #define USE_VALGRIND */
+#define USE_VALGRIND
 
 /*
  * Define this to cause pfree()'d memory to be cleared immediately, to
@@ -313,7 +313,7 @@
  * facilitate catching code that depends on the contents of uninitialized
  * memory.  Caution: this is horrendously expensive.
  */
-/* #define RANDOMIZE_ALLOCATED_MEMORY */
+#define RANDOMIZE_ALLOCATED_MEMORY
 
 /*
  * For cache invalidation debugging, define CLOBBER_CACHE_ENABLED to enable
-- 
2.17.0

  [text/x-diff] 0002-log_autovacuum_min_duration-is-LOGGING_WHAT.patch (5.3K, ../../20210404012546.GK6592@telsasoft.com/3-0002-log_autovacuum_min_duration-is-LOGGING_WHAT.patch)
  download | inline diff:
From 6604d8b515ef62e77fea2550e5f040d89fa948f4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:10:01 -0500
Subject: [PATCH 2/4] log_autovacuum_min_duration is LOGGING_WHAT

Not AUTOVACUUM, since 48f7e6439 and ef23a7744
---
 doc/src/sgml/config.sgml                      | 56 +++++++++----------
 src/backend/utils/misc/postgresql.conf.sample |  8 +--
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 9d87b5097a..c4d5126c2a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6688,6 +6688,34 @@ local0.*    /var/log/postgresql
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-log-autovacuum-min-duration" xreflabel="log_autovacuum_min_duration">
+      <term><varname>log_autovacuum_min_duration</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>log_autovacuum_min_duration</varname></primary>
+       <secondary>configuration parameter</secondary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Causes each action executed by autovacuum to be logged if it ran for at
+        least the specified amount of time.  Setting this to zero logs
+        all autovacuum actions. <literal>-1</literal> (the default) disables
+        logging autovacuum actions.
+        If this value is specified without units, it is taken as milliseconds.
+        For example, if you set this to
+        <literal>250ms</literal> then all automatic vacuums and analyzes that run
+        250ms or longer will be logged.  In addition, when this parameter is
+        set to any value other than <literal>-1</literal>, a message will be
+        logged if an autovacuum action is skipped due to a conflicting lock or a
+        concurrently dropped relation.  Enabling this parameter can be helpful
+        in tracking autovacuum activity.  This parameter can only be set in
+        the <filename>postgresql.conf</filename> file or on the server command line;
+        but the setting can be overridden for individual tables by
+        changing table storage parameters.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-log-checkpoints" xreflabel="log_checkpoints">
       <term><varname>log_checkpoints</varname> (<type>boolean</type>)
       <indexterm>
@@ -7662,34 +7690,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-log-autovacuum-min-duration" xreflabel="log_autovacuum_min_duration">
-      <term><varname>log_autovacuum_min_duration</varname> (<type>integer</type>)
-      <indexterm>
-       <primary><varname>log_autovacuum_min_duration</varname></primary>
-       <secondary>configuration parameter</secondary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Causes each action executed by autovacuum to be logged if it ran for at
-        least the specified amount of time.  Setting this to zero logs
-        all autovacuum actions. <literal>-1</literal> (the default) disables
-        logging autovacuum actions.
-        If this value is specified without units, it is taken as milliseconds.
-        For example, if you set this to
-        <literal>250ms</literal> then all automatic vacuums and analyzes that run
-        250ms or longer will be logged.  In addition, when this parameter is
-        set to any value other than <literal>-1</literal>, a message will be
-        logged if an autovacuum action is skipped due to a conflicting lock or a
-        concurrently dropped relation.  Enabling this parameter can be helpful
-        in tracking autovacuum activity.  This parameter can only be set in
-        the <filename>postgresql.conf</filename> file or on the server command line;
-        but the setting can be overridden for individual tables by
-        changing table storage parameters.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-autovacuum-max-workers" xreflabel="autovacuum_max_workers">
       <term><varname>autovacuum_max_workers</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 30cfddac1f..52c4373a79 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -568,6 +568,10 @@
 #log_temp_files = -1			# log temporary files equal or larger
 					# than the specified size in kilobytes;
 					# -1 disables, 0 logs all temp files
+#log_autovacuum_min_duration = -1	# -1 disables, 0 logs all actions and
+					# their durations, > 0 logs only
+					# actions running at least this number
+					# of milliseconds.
 #log_timezone = 'GMT'
 
 #------------------------------------------------------------------------------
@@ -608,10 +612,6 @@
 
 #autovacuum = on			# Enable autovacuum subprocess?  'on'
 					# requires track_counts to also be on.
-#log_autovacuum_min_duration = -1	# -1 disables, 0 logs all actions and
-					# their durations, > 0 logs only
-					# actions running at least this number
-					# of milliseconds.
 #autovacuum_max_workers = 3		# max number of autovacuum subprocesses
 					# (change requires restart)
 #autovacuum_naptime = 1min		# time between autovacuum runs
-- 
2.17.0

  [text/x-diff] 0003-track_commit_timestamp-is-REPLICATION_SENDING.patch (850B, ../../20210404012546.GK6592@telsasoft.com/4-0003-track_commit_timestamp-is-REPLICATION_SENDING.patch)
  download | inline diff:
From b386aceaece02b57b2c5c5640bbf38d9da99a5e4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:17:03 -0500
Subject: [PATCH 3/4] track_commit_timestamp is REPLICATION_SENDING

If I'm not wrong, this was missed at 4bd8ed31b
---
 src/backend/utils/misc/guc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c315dd4bc1..27287599ad 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1180,7 +1180,7 @@ static struct config_bool ConfigureNamesBool[] =
 		check_bonjour, NULL, NULL
 	},
 	{
-		{"track_commit_timestamp", PGC_POSTMASTER, REPLICATION,
+		{"track_commit_timestamp", PGC_POSTMASTER, REPLICATION_SENDING,
 			gettext_noop("Collects transaction commit time."),
 			NULL
 		},
-- 
2.17.0

  [text/x-diff] 0004-Change-force_parallel_mode-to-a-DEVELOPER-GUC-and-re.patch (7.3K, ../../20210404012546.GK6592@telsasoft.com/5-0004-Change-force_parallel_mode-to-a-DEVELOPER-GUC-and-re.patch)
  download | inline diff:
From cd224c1524d60c9b5679a853358b59dad00a35ca Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:24:50 -0500
Subject: [PATCH 4/4] Change force_parallel_mode to a DEVELOPER GUC, and remove
 it from sample config..

..to help avoid users finding this option and changing it in hopes that it'll
make their queries faster, but without reading the documentation or
understanding what it does.
---
 doc/src/sgml/config.sgml                      | 90 +++++++++----------
 src/backend/utils/misc/guc.c                  |  2 +-
 src/backend/utils/misc/postgresql.conf.sample |  1 -
 3 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c4d5126c2a..5c7c5a39ca 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5788,51 +5788,6 @@ SELECT * FROM parent WHERE key = 2400;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-force-parallel-mode" xreflabel="force_parallel_mode">
-      <term><varname>force_parallel_mode</varname> (<type>enum</type>)
-      <indexterm>
-       <primary><varname>force_parallel_mode</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Allows the use of parallel queries for testing purposes even in cases
-        where no performance benefit is expected.
-        The allowed values of <varname>force_parallel_mode</varname> are
-        <literal>off</literal> (use parallel mode only when it is expected to improve
-        performance), <literal>on</literal> (force parallel query for all queries
-        for which it is thought to be safe), and <literal>regress</literal> (like
-        <literal>on</literal>, but with additional behavior changes as explained
-        below).
-       </para>
-
-       <para>
-        More specifically, setting this value to <literal>on</literal> will add
-        a <literal>Gather</literal> node to the top of any query plan for which this
-        appears to be safe, so that the query runs inside of a parallel worker.
-        Even when a parallel worker is not available or cannot be used,
-        operations such as starting a subtransaction that would be prohibited
-        in a parallel query context will be prohibited unless the planner
-        believes that this will cause the query to fail.  If failures or
-        unexpected results occur when this option is set, some functions used
-        by the query may need to be marked <literal>PARALLEL UNSAFE</literal>
-        (or, possibly, <literal>PARALLEL RESTRICTED</literal>).
-       </para>
-
-       <para>
-        Setting this value to <literal>regress</literal> has all of the same effects
-        as setting it to <literal>on</literal> plus some additional effects that are
-        intended to facilitate automated regression testing.  Normally,
-        messages from a parallel worker include a context line indicating that,
-        but a setting of <literal>regress</literal> suppresses this line so that the
-        output is the same as in non-parallel execution.  Also,
-        the <literal>Gather</literal> nodes added to plans by this setting are hidden
-        in <literal>EXPLAIN</literal> output so that the output matches what
-        would be obtained if this setting were turned <literal>off</literal>.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-plan-cache_mode" xreflabel="plan_cache_mode">
       <term><varname>plan_cache_mode</varname> (<type>enum</type>)
       <indexterm>
@@ -10231,6 +10186,51 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-force-parallel-mode" xreflabel="force_parallel_mode">
+      <term><varname>force_parallel_mode</varname> (<type>enum</type>)
+      <indexterm>
+       <primary><varname>force_parallel_mode</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Allows the use of parallel queries for testing purposes even in cases
+        where no performance benefit is expected.
+        The allowed values of <varname>force_parallel_mode</varname> are
+        <literal>off</literal> (use parallel mode only when it is expected to improve
+        performance), <literal>on</literal> (force parallel query for all queries
+        for which it is thought to be safe), and <literal>regress</literal> (like
+        <literal>on</literal>, but with additional behavior changes as explained
+        below).
+       </para>
+
+       <para>
+        More specifically, setting this value to <literal>on</literal> will add
+        a <literal>Gather</literal> node to the top of any query plan for which this
+        appears to be safe, so that the query runs inside of a parallel worker.
+        Even when a parallel worker is not available or cannot be used,
+        operations such as starting a subtransaction that would be prohibited
+        in a parallel query context will be prohibited unless the planner
+        believes that this will cause the query to fail.  If failures or
+        unexpected results occur when this option is set, some functions used
+        by the query may need to be marked <literal>PARALLEL UNSAFE</literal>
+        (or, possibly, <literal>PARALLEL RESTRICTED</literal>).
+       </para>
+
+       <para>
+        Setting this value to <literal>regress</literal> has all of the same effects
+        as setting it to <literal>on</literal> plus some additional effects that are
+        intended to facilitate automated regression testing.  Normally,
+        messages from a parallel worker include a context line indicating that,
+        but a setting of <literal>regress</literal> suppresses this line so that the
+        output is the same as in non-parallel execution.  Also,
+        the <literal>Gather</literal> nodes added to plans by this setting are hidden
+        in <literal>EXPLAIN</literal> output so that the output matches what
+        would be obtained if this setting were turned <literal>off</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
       <term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
       <indexterm>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 27287599ad..06de7c0308 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4840,7 +4840,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER,
+		{"force_parallel_mode", PGC_USERSET, DEVELOPER_OPTIONS,
 			gettext_noop("Forces use of parallel query facilities."),
 			gettext_noop("If possible, run query using a parallel worker and with parallel restrictions."),
 			GUC_EXPLAIN
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 52c4373a79..50faa50462 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -415,7 +415,6 @@
 #from_collapse_limit = 8
 #join_collapse_limit = 8		# 1 disables collapsing of explicit
 					# JOIN clauses
-#force_parallel_mode = off
 #jit = on				# allow JIT compilation
 #plan_cache_mode = auto			# auto, force_generic_plan or
 					# force_custom_plan
-- 
2.17.0

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
@ 2021-04-08 21:38 ` Justin Pryzby <pryzby@telsasoft.com>
  1 sibling, 0 replies; 18+ messages in thread

From: Justin Pryzby @ 2021-04-08 21:38 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

The previous patches accidentally included some unrelated changes.

-- 
Justin

Attachments:

  [text/x-diff] v2-0001-track_activity_query_size-is-STATS_COLLECTOR-cate.patch (877B, ../../20210408213812.GA18734@telsasoft.com/2-v2-0001-track_activity_query_size-is-STATS_COLLECTOR-cate.patch)
  download | inline diff:
From fd67dd04d1b824a25e113796c235fd9fc9db23e0 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:06:37 -0500
Subject: [PATCH v2 1/4] track_activity_query_size is STATS_COLLECTOR category

Not Resource Usage / Memory, as since 995fb7420
---
 src/backend/utils/misc/guc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 090abdad8b..e54209995d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3506,7 +3506,7 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"track_activity_query_size", PGC_POSTMASTER, RESOURCES_MEM,
+		{"track_activity_query_size", PGC_POSTMASTER, STATS_COLLECTOR,
 			gettext_noop("Sets the size reserved for pg_stat_activity.query, in bytes."),
 			NULL,
 			GUC_UNIT_BYTE
-- 
2.17.0

  [text/x-diff] v2-0002-log_autovacuum_min_duration-is-LOGGING_WHAT.patch (5.3K, ../../20210408213812.GA18734@telsasoft.com/3-v2-0002-log_autovacuum_min_duration-is-LOGGING_WHAT.patch)
  download | inline diff:
From bbed9ed2c3c55b0ccd51358a5c62baa07d1a5ee1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:10:01 -0500
Subject: [PATCH v2 2/4] log_autovacuum_min_duration is LOGGING_WHAT

Not AUTOVACUUM, since 48f7e6439 and ef23a7744
---
 doc/src/sgml/config.sgml                      | 56 +++++++++----------
 src/backend/utils/misc/postgresql.conf.sample |  8 +--
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 26628f3e6d..eb154cd669 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6812,6 +6812,34 @@ local0.*    /var/log/postgresql
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-log-autovacuum-min-duration" xreflabel="log_autovacuum_min_duration">
+      <term><varname>log_autovacuum_min_duration</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>log_autovacuum_min_duration</varname></primary>
+       <secondary>configuration parameter</secondary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Causes each action executed by autovacuum to be logged if it ran for at
+        least the specified amount of time.  Setting this to zero logs
+        all autovacuum actions. <literal>-1</literal> (the default) disables
+        logging autovacuum actions.
+        If this value is specified without units, it is taken as milliseconds.
+        For example, if you set this to
+        <literal>250ms</literal> then all automatic vacuums and analyzes that run
+        250ms or longer will be logged.  In addition, when this parameter is
+        set to any value other than <literal>-1</literal>, a message will be
+        logged if an autovacuum action is skipped due to a conflicting lock or a
+        concurrently dropped relation.  Enabling this parameter can be helpful
+        in tracking autovacuum activity.  This parameter can only be set in
+        the <filename>postgresql.conf</filename> file or on the server command line;
+        but the setting can be overridden for individual tables by
+        changing table storage parameters.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-log-checkpoints" xreflabel="log_checkpoints">
       <term><varname>log_checkpoints</varname> (<type>boolean</type>)
       <indexterm>
@@ -7827,34 +7855,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-log-autovacuum-min-duration" xreflabel="log_autovacuum_min_duration">
-      <term><varname>log_autovacuum_min_duration</varname> (<type>integer</type>)
-      <indexterm>
-       <primary><varname>log_autovacuum_min_duration</varname></primary>
-       <secondary>configuration parameter</secondary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Causes each action executed by autovacuum to be logged if it ran for at
-        least the specified amount of time.  Setting this to zero logs
-        all autovacuum actions. <literal>-1</literal> (the default) disables
-        logging autovacuum actions.
-        If this value is specified without units, it is taken as milliseconds.
-        For example, if you set this to
-        <literal>250ms</literal> then all automatic vacuums and analyzes that run
-        250ms or longer will be logged.  In addition, when this parameter is
-        set to any value other than <literal>-1</literal>, a message will be
-        logged if an autovacuum action is skipped due to a conflicting lock or a
-        concurrently dropped relation.  Enabling this parameter can be helpful
-        in tracking autovacuum activity.  This parameter can only be set in
-        the <filename>postgresql.conf</filename> file or on the server command line;
-        but the setting can be overridden for individual tables by
-        changing table storage parameters.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-autovacuum-max-workers" xreflabel="autovacuum_max_workers">
       <term><varname>autovacuum_max_workers</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 9830cfe382..cc9edc410f 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -575,6 +575,10 @@
 #log_temp_files = -1			# log temporary files equal or larger
 					# than the specified size in kilobytes;
 					# -1 disables, 0 logs all temp files
+#log_autovacuum_min_duration = -1	# -1 disables, 0 logs all actions and
+					# their durations, > 0 logs only
+					# actions running at least this number
+					# of milliseconds.
 #log_timezone = 'GMT'
 
 #------------------------------------------------------------------------------
@@ -616,10 +620,6 @@
 
 #autovacuum = on			# Enable autovacuum subprocess?  'on'
 					# requires track_counts to also be on.
-#log_autovacuum_min_duration = -1	# -1 disables, 0 logs all actions and
-					# their durations, > 0 logs only
-					# actions running at least this number
-					# of milliseconds.
 #autovacuum_max_workers = 3		# max number of autovacuum subprocesses
 					# (change requires restart)
 #autovacuum_naptime = 1min		# time between autovacuum runs
-- 
2.17.0

  [text/x-diff] v2-0003-track_commit_timestamp-is-REPLICATION_SENDING.patch (853B, ../../20210408213812.GA18734@telsasoft.com/4-v2-0003-track_commit_timestamp-is-REPLICATION_SENDING.patch)
  download | inline diff:
From 7cc9fa15f08b7e95fb9d7a00b548e581be5bf40b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:17:03 -0500
Subject: [PATCH v2 3/4] track_commit_timestamp is REPLICATION_SENDING

If I'm not wrong, this was missed at 4bd8ed31b
---
 src/backend/utils/misc/guc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e54209995d..2b9583cc77 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1188,7 +1188,7 @@ static struct config_bool ConfigureNamesBool[] =
 		check_bonjour, NULL, NULL
 	},
 	{
-		{"track_commit_timestamp", PGC_POSTMASTER, REPLICATION,
+		{"track_commit_timestamp", PGC_POSTMASTER, REPLICATION_SENDING,
 			gettext_noop("Collects transaction commit time."),
 			NULL
 		},
-- 
2.17.0

  [text/x-diff] v2-0004-Change-force_parallel_mode-to-a-DEVELOPER-GUC-and.patch (7.3K, ../../20210408213812.GA18734@telsasoft.com/5-v2-0004-Change-force_parallel_mode-to-a-DEVELOPER-GUC-and.patch)
  download | inline diff:
From 0259b2e7d4b14e2c0c16004a619ac2275cf23c5b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:24:50 -0500
Subject: [PATCH v2 4/4] Change force_parallel_mode to a DEVELOPER GUC, and
 remove it from sample config..

..to help avoid users finding this option and changing it in hopes that it'll
make their queries faster, but without reading the documentation or
understanding what it does.
---
 doc/src/sgml/config.sgml                      | 90 +++++++++----------
 src/backend/utils/misc/guc.c                  |  2 +-
 src/backend/utils/misc/postgresql.conf.sample |  1 -
 3 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index eb154cd669..4568a5c5a0 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5912,51 +5912,6 @@ SELECT * FROM parent WHERE key = 2400;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-force-parallel-mode" xreflabel="force_parallel_mode">
-      <term><varname>force_parallel_mode</varname> (<type>enum</type>)
-      <indexterm>
-       <primary><varname>force_parallel_mode</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Allows the use of parallel queries for testing purposes even in cases
-        where no performance benefit is expected.
-        The allowed values of <varname>force_parallel_mode</varname> are
-        <literal>off</literal> (use parallel mode only when it is expected to improve
-        performance), <literal>on</literal> (force parallel query for all queries
-        for which it is thought to be safe), and <literal>regress</literal> (like
-        <literal>on</literal>, but with additional behavior changes as explained
-        below).
-       </para>
-
-       <para>
-        More specifically, setting this value to <literal>on</literal> will add
-        a <literal>Gather</literal> node to the top of any query plan for which this
-        appears to be safe, so that the query runs inside of a parallel worker.
-        Even when a parallel worker is not available or cannot be used,
-        operations such as starting a subtransaction that would be prohibited
-        in a parallel query context will be prohibited unless the planner
-        believes that this will cause the query to fail.  If failures or
-        unexpected results occur when this option is set, some functions used
-        by the query may need to be marked <literal>PARALLEL UNSAFE</literal>
-        (or, possibly, <literal>PARALLEL RESTRICTED</literal>).
-       </para>
-
-       <para>
-        Setting this value to <literal>regress</literal> has all of the same effects
-        as setting it to <literal>on</literal> plus some additional effects that are
-        intended to facilitate automated regression testing.  Normally,
-        messages from a parallel worker include a context line indicating that,
-        but a setting of <literal>regress</literal> suppresses this line so that the
-        output is the same as in non-parallel execution.  Also,
-        the <literal>Gather</literal> nodes added to plans by this setting are hidden
-        in <literal>EXPLAIN</literal> output so that the output matches what
-        would be obtained if this setting were turned <literal>off</literal>.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-plan-cache_mode" xreflabel="plan_cache_mode">
       <term><varname>plan_cache_mode</varname> (<type>enum</type>)
       <indexterm>
@@ -10462,6 +10417,51 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-force-parallel-mode" xreflabel="force_parallel_mode">
+      <term><varname>force_parallel_mode</varname> (<type>enum</type>)
+      <indexterm>
+       <primary><varname>force_parallel_mode</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Allows the use of parallel queries for testing purposes even in cases
+        where no performance benefit is expected.
+        The allowed values of <varname>force_parallel_mode</varname> are
+        <literal>off</literal> (use parallel mode only when it is expected to improve
+        performance), <literal>on</literal> (force parallel query for all queries
+        for which it is thought to be safe), and <literal>regress</literal> (like
+        <literal>on</literal>, but with additional behavior changes as explained
+        below).
+       </para>
+
+       <para>
+        More specifically, setting this value to <literal>on</literal> will add
+        a <literal>Gather</literal> node to the top of any query plan for which this
+        appears to be safe, so that the query runs inside of a parallel worker.
+        Even when a parallel worker is not available or cannot be used,
+        operations such as starting a subtransaction that would be prohibited
+        in a parallel query context will be prohibited unless the planner
+        believes that this will cause the query to fail.  If failures or
+        unexpected results occur when this option is set, some functions used
+        by the query may need to be marked <literal>PARALLEL UNSAFE</literal>
+        (or, possibly, <literal>PARALLEL RESTRICTED</literal>).
+       </para>
+
+       <para>
+        Setting this value to <literal>regress</literal> has all of the same effects
+        as setting it to <literal>on</literal> plus some additional effects that are
+        intended to facilitate automated regression testing.  Normally,
+        messages from a parallel worker include a context line indicating that,
+        but a setting of <literal>regress</literal> suppresses this line so that the
+        output is the same as in non-parallel execution.  Also,
+        the <literal>Gather</literal> nodes added to plans by this setting are hidden
+        in <literal>EXPLAIN</literal> output so that the output matches what
+        would be obtained if this setting were turned <literal>off</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
       <term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
       <indexterm>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2b9583cc77..598813da2d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4922,7 +4922,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER,
+		{"force_parallel_mode", PGC_USERSET, DEVELOPER_OPTIONS,
 			gettext_noop("Forces use of parallel query facilities."),
 			gettext_noop("If possible, run query using a parallel worker and with parallel restrictions."),
 			GUC_EXPLAIN
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index cc9edc410f..a38fe18886 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -421,7 +421,6 @@
 #from_collapse_limit = 8
 #join_collapse_limit = 8		# 1 disables collapsing of explicit
 					# JOIN clauses
-#force_parallel_mode = off
 #jit = on				# allow JIT compilation
 #plan_cache_mode = auto			# auto, force_generic_plan or
 					# force_custom_plan
-- 
2.17.0

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
@ 2021-04-09 01:50 ` Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-29 04:24   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  1 sibling, 2 replies; 18+ messages in thread

From: Michael Paquier @ 2021-04-09 01:50 UTC (permalink / raw)
  To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Bruce Momjian <bruce@momjian.us>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

On Sat, Apr 03, 2021 at 08:25:46PM -0500, Justin Pryzby wrote:
> Forking this thread
> https://www.postgresql.org/message-id/20210403154336.GG29125%40momjian.us

Didn't see this one, thanks for forking.

> I understood "developer" to mean someone who's debugging postgres itself, not
> (say) a function written using pl/pgsql.  Like backtrace_functions,
> post_auth_delay, jit_profiling_support.
> 
> But I see that some "dev" options are more user-facing (for a sufficiently
> advanced user):
> ignore_checksum_failure, ignore_invalid_pages, zero_damaged_pages.
> 
> Also, I understood this to mean the "category" in pg_settings, but I guess
> what's important here is the absense of the GUC in the sample/template config
> file.  pg_settings.category and the sample headings it appears are intended to
> be synchronized, but a few of them are out of sync.  See attached.
> 
> +1 to move this to "developer" options and remove it from the sample config:
> 
> # - Other Planner Options -
> #force_parallel_mode = off

0001 has some changes to pg_config_manual.h related to valgrind and
memory randomization.  You may want to remove that before posting a
patch.

-       {"track_commit_timestamp", PGC_POSTMASTER, REPLICATION,
+       {"track_commit_timestamp", PGC_POSTMASTER, REPLICATION_SENDING,
I can get behind this change for clarity where it gets actively used.

-       {"track_activity_query_size", PGC_POSTMASTER, RESOURCES_MEM,
+       {"track_activity_query_size", PGC_POSTMASTER, STATS_COLLECTOR,
But not this one, because it is a memory setting.

-       {"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER,
+       {"force_parallel_mode", PGC_USERSET, DEVELOPER_OPTIONS,
And not this one either, as it is mainly a planner thing, like the
other parameters in the same area.

The last change is related to log_autovacuum_min_duration, and I can
get behind the argument you are making to group all log activity
parameters together.  Now, about this part:
+#log_autovacuum_min_duration = -1  # -1 disables, 0 logs all actions and
+                   # their durations, > 0 logs only
+                   # actions running at least this number
+                   # of milliseconds.
I think that we should clarify in the description that this is an
autovacuum-only thing, say by appending a small sentence about the
fact that it logs autovacuum activities, in a similar fashion to
log_temp_files.  Moving the parameter out of the autovacuum section
makes it lose a bit of context.

@@ -6903,6 +6903,7 @@ fetch_more_data_begin(AsyncRequest *areq)
    char        sql[64];

    Assert(!fsstate->conn_state->pendingAreq);
+   Assert(fsstate->conn);
What's this diff doing here? 
--
Michaelx

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../YG+y%2FeJErKPDUhiS@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-09 03:17   ` Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  1 sibling, 1 reply; 18+ messages in thread

From: Justin Pryzby @ 2021-04-09 03:17 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Bruce Momjian <bruce@momjian.us>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

On Fri, Apr 09, 2021 at 10:50:53AM +0900, Michael Paquier wrote:
> On Sat, Apr 03, 2021 at 08:25:46PM -0500, Justin Pryzby wrote:
> > Forking this thread
> > https://www.postgresql.org/message-id/20210403154336.GG29125%40momjian.us
> 
> Didn't see this one, thanks for forking.
> 
> -       {"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER,
> +       {"force_parallel_mode", PGC_USERSET, DEVELOPER_OPTIONS,
> And not this one either, as it is mainly a planner thing, like the
> other parameters in the same area.

This is the main motive behind the patch.

Developer options aren't shown in postgresql.conf.sample, which it seems like
sometimes people read through quickly, setting a whole bunch of options that
sound good, sometimes including this one.  And in the best case they then ask
on -performance why their queries are slow and we tell them to turn it back off
to fix their issues.  This changes to no longer put it in .sample, and calling
it a "dev" option seems to be the classification and mechanism by which to do
that.

-- 
Justin

ps, Maybe you saw that I'd already resent without including the accidental junk
hunks.





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
@ 2021-04-09 11:39     ` Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Bruce Momjian @ 2021-04-09 11:39 UTC (permalink / raw)
  To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

On Thu, Apr  8, 2021 at 10:17:18PM -0500, Justin Pryzby wrote:
> On Fri, Apr 09, 2021 at 10:50:53AM +0900, Michael Paquier wrote:
> > On Sat, Apr 03, 2021 at 08:25:46PM -0500, Justin Pryzby wrote:
> > > Forking this thread
> > > https://www.postgresql.org/message-id/20210403154336.GG29125%40momjian.us
> > 
> > Didn't see this one, thanks for forking.
> > 
> > -       {"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER,
> > +       {"force_parallel_mode", PGC_USERSET, DEVELOPER_OPTIONS,
> > And not this one either, as it is mainly a planner thing, like the
> > other parameters in the same area.
> 
> This is the main motive behind the patch.
> 
> Developer options aren't shown in postgresql.conf.sample, which it seems like
> sometimes people read through quickly, setting a whole bunch of options that
> sound good, sometimes including this one.  And in the best case they then ask
> on -performance why their queries are slow and we tell them to turn it back off
> to fix their issues.  This changes to no longer put it in .sample, and calling
> it a "dev" option seems to be the classification and mechanism by which to do
> that.

+1

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.






^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
@ 2021-04-12 05:01       ` Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:28         ` Re: [PATCH] force_parallel_mode and GUC categories Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 18+ messages in thread

From: Michael Paquier @ 2021-04-12 05:01 UTC (permalink / raw)
  To: Bruce Momjian <bruce@momjian.us>; +Cc: Justin Pryzby <pryzby@telsasoft.com>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

On Fri, Apr 09, 2021 at 07:39:28AM -0400, Bruce Momjian wrote:
> On Thu, Apr  8, 2021 at 10:17:18PM -0500, Justin Pryzby wrote:
>> This is the main motive behind the patch.
>> 
>> Developer options aren't shown in postgresql.conf.sample, which it seems like
>> sometimes people read through quickly, setting a whole bunch of options that
>> sound good, sometimes including this one.  And in the best case they then ask
>> on -performance why their queries are slow and we tell them to turn it back off
>> to fix their issues.  This changes to no longer put it in .sample, and calling
>> it a "dev" option seems to be the classification and mechanism by which to do
>> that.
> 
> +1

Hm.  I can see the point you are making based on the bug report that
has led to this thread:
https://www.postgresql.org/message-id/CAN0SRDFV=Fv0zXHCGbh7gh=MTfw05Xd1x7gjJrZs5qn-TEphOw@mail.gmail...

However, I'd like to think that we can do better than what's proposed
in the patch.  There are a couple of things to consider here:
- Should the parameter be renamed to reflect that it should only be
used for testing purposes?
- Should we make more general the description of the developer options
in the docs?

I have applied the patch for log_autovacuum_min_duration for now, as
this one is clearly wrong.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../YHPUHdXuCzMIROf9@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-12 05:28         ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
  1 sibling, 0 replies; 18+ messages in thread

From: Bharath Rupireddy @ 2021-04-12 05:28 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Bruce Momjian <bruce@momjian.us>; Justin Pryzby <pryzby@telsasoft.com>; Tom Lane <tgl@sss.pgh.pa.us>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

On Mon, Apr 12, 2021 at 10:31 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Fri, Apr 09, 2021 at 07:39:28AM -0400, Bruce Momjian wrote:
> > On Thu, Apr  8, 2021 at 10:17:18PM -0500, Justin Pryzby wrote:
> >> This is the main motive behind the patch.
> >>
> >> Developer options aren't shown in postgresql.conf.sample, which it seems like
> >> sometimes people read through quickly, setting a whole bunch of options that
> >> sound good, sometimes including this one.  And in the best case they then ask
> >> on -performance why their queries are slow and we tell them to turn it back off
> >> to fix their issues.  This changes to no longer put it in .sample, and calling
> >> it a "dev" option seems to be the classification and mechanism by which to do
> >> that.
> >
> > +1
>
> Hm.  I can see the point you are making based on the bug report that
> has led to this thread:
> https://www.postgresql.org/message-id/CAN0SRDFV=Fv0zXHCGbh7gh=MTfw05Xd1x7gjJrZs5qn-TEphOw@mail.gmail...
>
> However, I'd like to think that we can do better than what's proposed
> in the patch.  There are a couple of things to consider here:
> - Should the parameter be renamed to reflect that it should only be
> used for testing purposes?
> - Should we make more general the description of the developer options
> in the docs?

IMO, categorizing force_parallel_mode to DEVELOPER_OPTIONS and moving
it to the "Developer Options" section in config.sgml looks
appropriate. So, the v2-0004 patch proposed by Justin at [1] looks
good to me. If there are any other GUCs that are not meant to be used
in production, IMO we could follow the same.

[1] https://www.postgresql.org/message-id/20210408213812.GA18734%40telsasoft.com

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-12 05:40         ` Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  1 sibling, 1 reply; 18+ messages in thread

From: Tom Lane @ 2021-04-12 05:40 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Bruce Momjian <bruce@momjian.us>; Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> However, I'd like to think that we can do better than what's proposed
> in the patch.  There are a couple of things to consider here:
> - Should the parameter be renamed to reflect that it should only be
> used for testing purposes?

-1 to that part, because it would break a bunch of buildfarm animals'
configurations.  I doubt that any gain in clarity would be worth it.

> - Should we make more general the description of the developer options
> in the docs?

Perhaps ... what did you have in mind?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-13 07:34           ` Michael Paquier <michael@paquier.xyz>
  2021-04-13 12:31             ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-13 14:12             ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 2 replies; 18+ messages in thread

From: Michael Paquier @ 2021-04-13 07:34 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bruce Momjian <bruce@momjian.us>; Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers@lists.postgresql.org

On Mon, Apr 12, 2021 at 01:40:52AM -0400, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> However, I'd like to think that we can do better than what's proposed
>> in the patch.  There are a couple of things to consider here:
>> - Should the parameter be renamed to reflect that it should only be
>> used for testing purposes?
> 
> -1 to that part, because it would break a bunch of buildfarm animals'
> configurations.  I doubt that any gain in clarity would be worth it.

Okay.

>> - Should we make more general the description of the developer options
>> in the docs?
> 
> Perhaps ... what did you have in mind?

The first sentence of the page now says that:
"The following parameters are intended for work on the PostgreSQL
source code, and in some cases to assist with recovery of severely
damaged databases."

That does not stick with force_parallel_mode IMO.  Maybe:
"The following parameters are intended for development work related to
PostgreSQL.  Some of them work on the PostgreSQL source code, some of
them can be used to control the run-time behavior of the server, and
in some cases they can be used to assist with the recovery of severely
damaged databases."
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../YHVJfw2YN5%2FoGAcQ@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-13 12:31             ` Justin Pryzby <pryzby@telsasoft.com>
  2021-04-14 06:57               ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  1 sibling, 1 reply; 18+ messages in thread

From: Justin Pryzby @ 2021-04-13 12:31 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers@lists.postgresql.org

On Tue, Apr 13, 2021 at 04:34:23PM +0900, Michael Paquier wrote:
> On Mon, Apr 12, 2021 at 01:40:52AM -0400, Tom Lane wrote:
> >> - Should we make more general the description of the developer options
> >> in the docs?
> > 
> > Perhaps ... what did you have in mind?
> 
> The first sentence of the page now says that:
> "The following parameters are intended for work on the PostgreSQL
> source code, and in some cases to assist with recovery of severely
> damaged databases."
> 
> That does not stick with force_parallel_mode IMO.  Maybe:

Good point.

-- 
Justin

Attachments:

  [text/x-diff] v3-0001-track_activity_query_size-is-STATS_COLLECTOR-cate.patch (877B, ../../20210413123139.GE6091@telsasoft.com/2-v3-0001-track_activity_query_size-is-STATS_COLLECTOR-cate.patch)
  download | inline diff:
From b5355f3384df1aab87e4cb89afcf4d9703c14bdb Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:06:37 -0500
Subject: [PATCH v3 1/3] track_activity_query_size is STATS_COLLECTOR category

Not Resource Usage / Memory, as since 995fb7420
---
 src/backend/utils/misc/guc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index d0a51b507d..e65d62e71d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3506,7 +3506,7 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"track_activity_query_size", PGC_POSTMASTER, RESOURCES_MEM,
+		{"track_activity_query_size", PGC_POSTMASTER, STATS_COLLECTOR,
 			gettext_noop("Sets the size reserved for pg_stat_activity.query, in bytes."),
 			NULL,
 			GUC_UNIT_BYTE
-- 
2.17.0

  [text/x-diff] v3-0002-track_commit_timestamp-is-REPLICATION_SENDING.patch (853B, ../../20210413123139.GE6091@telsasoft.com/3-v3-0002-track_commit_timestamp-is-REPLICATION_SENDING.patch)
  download | inline diff:
From b491b4520f157fb1328925f500f15fe2ef8e457c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:17:03 -0500
Subject: [PATCH v3 2/3] track_commit_timestamp is REPLICATION_SENDING

If I'm not wrong, this was missed at 4bd8ed31b
---
 src/backend/utils/misc/guc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e65d62e71d..a3c2ebbc53 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1188,7 +1188,7 @@ static struct config_bool ConfigureNamesBool[] =
 		check_bonjour, NULL, NULL
 	},
 	{
-		{"track_commit_timestamp", PGC_POSTMASTER, REPLICATION,
+		{"track_commit_timestamp", PGC_POSTMASTER, REPLICATION_SENDING,
 			gettext_noop("Collects transaction commit time."),
 			NULL
 		},
-- 
2.17.0

  [text/x-diff] v3-0003-Change-force_parallel_mode-to-a-DEVELOPER-GUC-and.patch (8.0K, ../../20210413123139.GE6091@telsasoft.com/4-v3-0003-Change-force_parallel_mode-to-a-DEVELOPER-GUC-and.patch)
  download | inline diff:
From e9b556f2bc75dc8350759e4b32e588325086218e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 3 Apr 2021 19:24:50 -0500
Subject: [PATCH v3 3/3] Change force_parallel_mode to a DEVELOPER GUC, and
 remove it from sample config..

..to discourage users from changing this option in hopes that it'll make their
queries faster, but without reading the documentation or understanding what it
does.
---
 doc/src/sgml/config.sgml                      | 96 ++++++++++---------
 src/backend/utils/misc/guc.c                  |  2 +-
 src/backend/utils/misc/postgresql.conf.sample |  1 -
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f749fe9ce7..a7350c6ddc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5912,51 +5912,6 @@ SELECT * FROM parent WHERE key = 2400;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-force-parallel-mode" xreflabel="force_parallel_mode">
-      <term><varname>force_parallel_mode</varname> (<type>enum</type>)
-      <indexterm>
-       <primary><varname>force_parallel_mode</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Allows the use of parallel queries for testing purposes even in cases
-        where no performance benefit is expected.
-        The allowed values of <varname>force_parallel_mode</varname> are
-        <literal>off</literal> (use parallel mode only when it is expected to improve
-        performance), <literal>on</literal> (force parallel query for all queries
-        for which it is thought to be safe), and <literal>regress</literal> (like
-        <literal>on</literal>, but with additional behavior changes as explained
-        below).
-       </para>
-
-       <para>
-        More specifically, setting this value to <literal>on</literal> will add
-        a <literal>Gather</literal> node to the top of any query plan for which this
-        appears to be safe, so that the query runs inside of a parallel worker.
-        Even when a parallel worker is not available or cannot be used,
-        operations such as starting a subtransaction that would be prohibited
-        in a parallel query context will be prohibited unless the planner
-        believes that this will cause the query to fail.  If failures or
-        unexpected results occur when this option is set, some functions used
-        by the query may need to be marked <literal>PARALLEL UNSAFE</literal>
-        (or, possibly, <literal>PARALLEL RESTRICTED</literal>).
-       </para>
-
-       <para>
-        Setting this value to <literal>regress</literal> has all of the same effects
-        as setting it to <literal>on</literal> plus some additional effects that are
-        intended to facilitate automated regression testing.  Normally,
-        messages from a parallel worker include a context line indicating that,
-        but a setting of <literal>regress</literal> suppresses this line so that the
-        output is the same as in non-parallel execution.  Also,
-        the <literal>Gather</literal> nodes added to plans by this setting are hidden
-        in <literal>EXPLAIN</literal> output so that the output matches what
-        would be obtained if this setting were turned <literal>off</literal>.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-plan-cache_mode" xreflabel="plan_cache_mode">
       <term><varname>plan_cache_mode</varname> (<type>enum</type>)
       <indexterm>
@@ -10374,8 +10329,10 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
     <title>Developer Options</title>
 
     <para>
-     The following parameters are intended for work on the
-     <productname>PostgreSQL</productname> source code, and in some cases
+     The following parameters are intended for development with
+     <productname>PostgreSQL</productname>.  Some of them are useful while
+     writing SQL queries, some of them are useful when
+     working on the source code of PostgreSQL itself, and
      to assist with recovery of severely damaged databases.  There
      should be no reason to use them on a production database.
      As such, they have been excluded from the sample
@@ -10464,6 +10421,51 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-force-parallel-mode" xreflabel="force_parallel_mode">
+      <term><varname>force_parallel_mode</varname> (<type>enum</type>)
+      <indexterm>
+       <primary><varname>force_parallel_mode</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Allows the use of parallel queries for testing purposes even in cases
+        where no performance benefit is expected.
+        The allowed values of <varname>force_parallel_mode</varname> are
+        <literal>off</literal> (use parallel mode only when it is expected to improve
+        performance), <literal>on</literal> (force parallel query for all queries
+        for which it is thought to be safe), and <literal>regress</literal> (like
+        <literal>on</literal>, but with additional behavior changes as explained
+        below).
+       </para>
+
+       <para>
+        More specifically, setting this value to <literal>on</literal> will add
+        a <literal>Gather</literal> node to the top of any query plan for which this
+        appears to be safe, so that the query runs inside of a parallel worker.
+        Even when a parallel worker is not available or cannot be used,
+        operations such as starting a subtransaction that would be prohibited
+        in a parallel query context will be prohibited unless the planner
+        believes that this will cause the query to fail.  If failures or
+        unexpected results occur when this option is set, some functions used
+        by the query may need to be marked <literal>PARALLEL UNSAFE</literal>
+        (or, possibly, <literal>PARALLEL RESTRICTED</literal>).
+       </para>
+
+       <para>
+        Setting this value to <literal>regress</literal> has all of the same effects
+        as setting it to <literal>on</literal> plus some additional effects that are
+        intended to facilitate automated regression testing.  Normally,
+        messages from a parallel worker include a context line indicating that,
+        but a setting of <literal>regress</literal> suppresses this line so that the
+        output is the same as in non-parallel execution.  Also,
+        the <literal>Gather</literal> nodes added to plans by this setting are hidden
+        in <literal>EXPLAIN</literal> output so that the output matches what
+        would be obtained if this setting were turned <literal>off</literal>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
       <term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
       <indexterm>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a3c2ebbc53..b8738561e6 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4922,7 +4922,7 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER,
+		{"force_parallel_mode", PGC_USERSET, DEVELOPER_OPTIONS,
 			gettext_noop("Forces use of parallel query facilities."),
 			gettext_noop("If possible, run query using a parallel worker and with parallel restrictions."),
 			GUC_EXPLAIN
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2f6dd014a8..0f7f49b949 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -421,7 +421,6 @@
 #from_collapse_limit = 8
 #join_collapse_limit = 8		# 1 disables collapsing of explicit
 					# JOIN clauses
-#force_parallel_mode = off
 #jit = on				# allow JIT compilation
 #plan_cache_mode = auto			# auto, force_generic_plan or
 					# force_custom_plan
-- 
2.17.0

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-13 12:31             ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
@ 2021-04-14 06:57               ` Michael Paquier <michael@paquier.xyz>
  2021-04-23 18:23                 ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2021-04-14 06:57 UTC (permalink / raw)
  To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers@lists.postgresql.org

On Tue, Apr 13, 2021 at 07:31:39AM -0500, Justin Pryzby wrote:
> Good point.

Thanks.  I have used the wording that Tom has proposed upthread, added
one GUC_NOT_IN_SAMPLE that you forgot, and applied the
force_parallel_mode patch.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../YHaSUbjPeh1XesoJ@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-13 12:31             ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-14 06:57               ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-23 18:23                 ` Justin Pryzby <pryzby@telsasoft.com>
  2021-04-24 01:50                   ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 18+ messages in thread

From: Justin Pryzby @ 2021-04-23 18:23 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers@lists.postgresql.org

On Wed, Apr 14, 2021 at 03:57:21PM +0900, Michael Paquier wrote:
> On Tue, Apr 13, 2021 at 07:31:39AM -0500, Justin Pryzby wrote:
> > Good point.
> 
> Thanks.  I have used the wording that Tom has proposed upthread, added
> one GUC_NOT_IN_SAMPLE that you forgot, and applied the
> force_parallel_mode patch.

Thanks.  It just occured to me to ask if we should backpatch it.
The goal is to avoid someone trying to use this as a peformance option.

It's to their benefit and ours if they don't do that on v10-13 for the next 5
years, not just v14-17.

The patch seems to apply cleanly on v12 but cherry-pick needs help for other
branches...

-- 
Justin





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-13 12:31             ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-14 06:57               ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-23 18:23                 ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
@ 2021-04-24 01:50                   ` Michael Paquier <michael@paquier.xyz>
  2021-04-24 02:57                     ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  0 siblings, 1 reply; 18+ messages in thread

From: Michael Paquier @ 2021-04-24 01:50 UTC (permalink / raw)
  To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers@lists.postgresql.org

On Fri, Apr 23, 2021 at 01:23:26PM -0500, Justin Pryzby wrote:
> The patch seems to apply cleanly on v12 but cherry-pick needs help for other
> branches...

FWIW, this did not seem bad enough to me to require a back-patch.
This parameter got introduced in 2016 and this was the only report
related to it for the last 5 years.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../YIN5XQgI4YhPyKLh@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-13 12:31             ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-14 06:57               ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-23 18:23                 ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-24 01:50                   ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-24 02:57                     ` Justin Pryzby <pryzby@telsasoft.com>
  0 siblings, 0 replies; 18+ messages in thread

From: Justin Pryzby @ 2021-04-24 02:57 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers@lists.postgresql.org

On Sat, Apr 24, 2021 at 10:50:21AM +0900, Michael Paquier wrote:
> On Fri, Apr 23, 2021 at 01:23:26PM -0500, Justin Pryzby wrote:
> > The patch seems to apply cleanly on v12 but cherry-pick needs help for other
> > branches...
> 
> FWIW, this did not seem bad enough to me to require a back-patch.
> This parameter got introduced in 2016 and this was the only report
> related to it for the last 5 years.

No, it's not the first report - although I'm surprised I wasn't able to find
more than these.

https://www.postgresql.org/message-id/20190102164525.GU25379@telsasoft.com
https://www.postgresql.org/message-id/CAKJS1f_Qi0iboCos3wu6QiAbdF-9FoK57wxzKbe2-WcesN4rFA%40mail.gma...

-- 
Justin





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-13 14:12             ` Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-14 04:54               ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  1 sibling, 1 reply; 18+ messages in thread

From: Tom Lane @ 2021-04-13 14:12 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Bruce Momjian <bruce@momjian.us>; Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers@lists.postgresql.org

Michael Paquier <michael@paquier.xyz> writes:
> On Mon, Apr 12, 2021 at 01:40:52AM -0400, Tom Lane wrote:
>> Perhaps ... what did you have in mind?

> The first sentence of the page now says that:
> "The following parameters are intended for work on the PostgreSQL
> source code, and in some cases to assist with recovery of severely
> damaged databases."

> That does not stick with force_parallel_mode IMO.  Maybe:
> "The following parameters are intended for development work related to
> PostgreSQL.  Some of them work on the PostgreSQL source code, some of
> them can be used to control the run-time behavior of the server, and
> in some cases they can be used to assist with the recovery of severely
> damaged databases."

I think that's overly wordy.  Maybe

The following parameters are intended for developer testing, and
should never be enabled for production work.  However, some of
them can be used to assist with the recovery of severely
damaged databases.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-09 03:17   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 11:39     ` Re: [PATCH] force_parallel_mode and GUC categories Bruce Momjian <bruce@momjian.us>
  2021-04-12 05:01       ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-12 05:40         ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  2021-04-13 07:34           ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-13 14:12             ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
@ 2021-04-14 04:54               ` Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 18+ messages in thread

From: Michael Paquier @ 2021-04-14 04:54 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bruce Momjian <bruce@momjian.us>; Justin Pryzby <pryzby@telsasoft.com>; pgsql-hackers@lists.postgresql.org

On Tue, Apr 13, 2021 at 10:12:35AM -0400, Tom Lane wrote:
> The following parameters are intended for developer testing, and
> should never be enabled for production work.  However, some of
> them can be used to assist with the recovery of severely
> damaged databases.

Okay, that's fine by me.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../YHZ1ipDBc4jaVNAX@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
@ 2021-04-29 04:24   ` Justin Pryzby <pryzby@telsasoft.com>
  2021-05-08 16:39     ` Re: [PATCH] force_parallel_mode and GUC categories Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 1 reply; 18+ messages in thread

From: Justin Pryzby @ 2021-04-29 04:24 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Bruce Momjian <bruce@momjian.us>; Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

On Fri, Apr 09, 2021 at 10:50:53AM +0900, Michael Paquier wrote:
> -       {"track_commit_timestamp", PGC_POSTMASTER, REPLICATION,
> +       {"track_commit_timestamp", PGC_POSTMASTER, REPLICATION_SENDING,
> I can get behind this change for clarity where it gets actively used.

I'm not sure what you meant?

...but, I realized just now that *zero* other GUCs use "REPLICATION".
And the documentation puts it in 20.6.1. Sending Servers,
so it still seems to me that this is correct to move this, too.

https://www.postgresql.org/docs/devel/runtime-config-replication.html

Then, I wonder if REPLICATION should be removed from guc_tables.h...

-- 
Justin





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: [PATCH] force_parallel_mode and GUC categories
  2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
  2021-04-09 01:50 ` Re: [PATCH] force_parallel_mode and GUC categories Michael Paquier <michael@paquier.xyz>
  2021-04-29 04:24   ` Re: [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
@ 2021-05-08 16:39     ` Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 18+ messages in thread

From: Tom Lane @ 2021-05-08 16:39 UTC (permalink / raw)
  To: Justin Pryzby <pryzby@telsasoft.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Bruce Momjian <bruce@momjian.us>; pgsql-hackers@lists.postgresql.org

Justin Pryzby <pryzby@telsasoft.com> writes:
> ...but, I realized just now that *zero* other GUCs use "REPLICATION".
> And the documentation puts it in 20.6.1. Sending Servers,
> so it still seems to me that this is correct to move this, too.
> https://www.postgresql.org/docs/devel/runtime-config-replication.html
> Then, I wonder if REPLICATION should be removed from guc_tables.h...

For the archives' sake --- these things are now committed as part of
a55a98477.  I'd forgotten this thread, and then rediscovered the same
inconsistencies as Justin had while reviewing Bharath Rupireddy's patch
for bug #16997 [1].

I think this thread can now be closed off as done.  However, there
are some open issues mentioned in the other thread, if anyone here
wants to comment.

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/16997-ff16127f6e0d1390%40postgresql.org





^ permalink  raw  reply  [nested|flat] 18+ messages in thread


end of thread, other threads:[~2021-05-08 16:39 UTC | newest]

Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-04-04 01:25 [PATCH] force_parallel_mode and GUC categories Justin Pryzby <pryzby@telsasoft.com>
2021-04-08 21:38 ` Justin Pryzby <pryzby@telsasoft.com>
2021-04-09 01:50 ` Michael Paquier <michael@paquier.xyz>
2021-04-09 03:17   ` Justin Pryzby <pryzby@telsasoft.com>
2021-04-09 11:39     ` Bruce Momjian <bruce@momjian.us>
2021-04-12 05:01       ` Michael Paquier <michael@paquier.xyz>
2021-04-12 05:28         ` Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
2021-04-12 05:40         ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-13 07:34           ` Michael Paquier <michael@paquier.xyz>
2021-04-13 12:31             ` Justin Pryzby <pryzby@telsasoft.com>
2021-04-14 06:57               ` Michael Paquier <michael@paquier.xyz>
2021-04-23 18:23                 ` Justin Pryzby <pryzby@telsasoft.com>
2021-04-24 01:50                   ` Michael Paquier <michael@paquier.xyz>
2021-04-24 02:57                     ` Justin Pryzby <pryzby@telsasoft.com>
2021-04-13 14:12             ` Tom Lane <tgl@sss.pgh.pa.us>
2021-04-14 04:54               ` Michael Paquier <michael@paquier.xyz>
2021-04-29 04:24   ` Justin Pryzby <pryzby@telsasoft.com>
2021-05-08 16:39     ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox