Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lSrWV-0004CJ-J7 for pgsql-hackers@arkaria.postgresql.org; Sun, 04 Apr 2021 01:25:55 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lSrWU-0000Vq-Ea for pgsql-hackers@arkaria.postgresql.org; Sun, 04 Apr 2021 01:25:54 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lSrWT-0000Vh-Qf for pgsql-hackers@lists.postgresql.org; Sun, 04 Apr 2021 01:25:54 +0000 Received: from mail-io1-xd30.google.com ([2607:f8b0:4864:20::d30]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1lSrWQ-0005BI-5H for pgsql-hackers@lists.postgresql.org; Sun, 04 Apr 2021 01:25:52 +0000 Received: by mail-io1-xd30.google.com with SMTP id j26so3665950iog.13 for ; Sat, 03 Apr 2021 18:25:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telsasoft-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=yyo7Aeq+RPqofP9/Z0oySwAuKHpap/XbHpfLVqo6hVc=; b=zrZEkLUZsrIhGmEhqqV6ynsWvzZfDeGJxPswmXmEsR2aHg5NTvVkae9Bg3VGKnE3Tq eTKn5vmrndv0ClBiHW2fPeYvNdt6+OxFNCQs/8XuNyc0M/G6yn+ycjw8Y+6HQTm7FvYy VBYylLAmGhs6v5Ewm9/EoYb8OXugLmjR4+OPxxNQtVydfjrIUD0h8EDa6SqQ8I1Uu13U 70an/uF1moiQ33mLC4t4aAklMS2HYb7FWfUy5nMM0pbwECuoptDy+TxXhnI5OwpLMbrV ATCMAKPTUasaMwRJg4bnE+W2ZlVBhxQZFPOHzPgrUP/nE/qipjntuy3wrCmokL67VlKE OlYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=yyo7Aeq+RPqofP9/Z0oySwAuKHpap/XbHpfLVqo6hVc=; b=UU7XziObnPejE8zRBlmGzgidjfn2NubNUBh1KBw4r4M3QlcMv0egCJHNo1a/21BWZG 76yGKh5PXGGMsmEAzkcbClthmr7njKvcq1t0cfKUTlOmvbockVCvq0WdCv0SnOB++g5C J2zgN39XWiG7/GIfb6oIh3nv2lLbLAiz4nezYExBwh43sya+I9cDCLXXhtraMb3peoaH K4BcB/bSHGBgdEsnokIso/bGjiQ4ms6DXrP+kzaZfgfOGhmhDq5PL/nj00zHx5rBoqtW vWOL7Yon+6X+9ZKfoEfG1eNCOigJTAg0E+c1Rdn9XO76sR9g24zeoFbHlKxAkZnhDh4K MJSQ== X-Gm-Message-State: AOAM533y4nQuibX/Xe+Bp43gkxxAZGBBMcCI4k4kmQElCmptAozv+kFQ U5dur0QKQUo+MfyRdQbmRPoGRl/OWecazg== X-Google-Smtp-Source: ABdhPJwc5zmFnWDrEArB5u5TSHNlEvrpEot2FcZLAUKnCNAb4M0pJcEw6L30EYiTNinYQFAD+1FZ0w== X-Received: by 2002:a5e:8416:: with SMTP id h22mr15612268ioj.119.1617499548370; Sat, 03 Apr 2021 18:25:48 -0700 (PDT) Received: from pryzbyj.telsasoft (charmander.telsasoft.com. [50.244.222.1]) by smtp.gmail.com with ESMTPSA id d3sm7970920ilv.80.2021.04.03.18.25.47 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sat, 03 Apr 2021 18:25:47 -0700 (PDT) Received: by pryzbyj.telsasoft (Postfix, from userid 1000) id C246F801559; Sat, 3 Apr 2021 20:25:46 -0500 (CDT) Date: Sat, 3 Apr 2021 20:25:46 -0500 From: Justin Pryzby To: Bruce Momjian Cc: Tom Lane , pgsql-hackers@lists.postgresql.org Subject: [PATCH] force_parallel_mode and GUC categories Message-ID: <20210404012546.GK6592@telsasoft.com> References: <20210403141651.GI6592@telsasoft.com> <20210403151201.GD29125@momjian.us> <3972636.1617464359@sss.pgh.pa.us> <20210403154114.GJ6592@telsasoft.com> <20210403154336.GG29125@momjian.us> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="GyRA7555PLgSTuth" Content-Disposition: inline In-Reply-To: <20210403154336.GG29125@momjian.us> User-Agent: Mutt/1.9.4 (2018-02-28) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --GyRA7555PLgSTuth Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 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 --GyRA7555PLgSTuth Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-track_activity_query_size-is-STATS_COLLECTOR-categor.patch" From ce0c5b99650859bb13f204ea56221ca854a601e1 Mon Sep 17 00:00:00 2001 From: Justin Pryzby 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 --GyRA7555PLgSTuth Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0002-log_autovacuum_min_duration-is-LOGGING_WHAT.patch" From 6604d8b515ef62e77fea2550e5f040d89fa948f4 Mon Sep 17 00:00:00 2001 From: Justin Pryzby 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 + + log_autovacuum_min_duration (integer) + + log_autovacuum_min_duration + configuration parameter + + + + + 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. -1 (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 + 250ms 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 -1, 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 postgresql.conf file or on the server command line; + but the setting can be overridden for individual tables by + changing table storage parameters. + + + + log_checkpoints (boolean) @@ -7662,34 +7690,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; - - log_autovacuum_min_duration (integer) - - log_autovacuum_min_duration - configuration parameter - - - - - 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. -1 (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 - 250ms 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 -1, 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 postgresql.conf file or on the server command line; - but the setting can be overridden for individual tables by - changing table storage parameters. - - - - autovacuum_max_workers (integer) 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 --GyRA7555PLgSTuth Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-track_commit_timestamp-is-REPLICATION_SENDING.patch" From b386aceaece02b57b2c5c5640bbf38d9da99a5e4 Mon Sep 17 00:00:00 2001 From: Justin Pryzby 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 --GyRA7555PLgSTuth Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0004-Change-force_parallel_mode-to-a-DEVELOPER-GUC-and-re.patch" From cd224c1524d60c9b5679a853358b59dad00a35ca Mon Sep 17 00:00:00 2001 From: Justin Pryzby 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; - - force_parallel_mode (enum) - - force_parallel_mode configuration parameter - - - - - Allows the use of parallel queries for testing purposes even in cases - where no performance benefit is expected. - The allowed values of force_parallel_mode are - off (use parallel mode only when it is expected to improve - performance), on (force parallel query for all queries - for which it is thought to be safe), and regress (like - on, but with additional behavior changes as explained - below). - - - - More specifically, setting this value to on will add - a Gather 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 PARALLEL UNSAFE - (or, possibly, PARALLEL RESTRICTED). - - - - Setting this value to regress has all of the same effects - as setting it to on 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 regress suppresses this line so that the - output is the same as in non-parallel execution. Also, - the Gather nodes added to plans by this setting are hidden - in EXPLAIN output so that the output matches what - would be obtained if this setting were turned off. - - - - plan_cache_mode (enum) @@ -10231,6 +10186,51 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' + + force_parallel_mode (enum) + + force_parallel_mode configuration parameter + + + + + Allows the use of parallel queries for testing purposes even in cases + where no performance benefit is expected. + The allowed values of force_parallel_mode are + off (use parallel mode only when it is expected to improve + performance), on (force parallel query for all queries + for which it is thought to be safe), and regress (like + on, but with additional behavior changes as explained + below). + + + + More specifically, setting this value to on will add + a Gather 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 PARALLEL UNSAFE + (or, possibly, PARALLEL RESTRICTED). + + + + Setting this value to regress has all of the same effects + as setting it to on 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 regress suppresses this line so that the + output is the same as in non-parallel execution. Also, + the Gather nodes added to plans by this setting are hidden + in EXPLAIN output so that the output matches what + would be obtained if this setting were turned off. + + + + ignore_system_indexes (boolean) 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 --GyRA7555PLgSTuth--