This is just a rebase.

As stated before, I added some information to the error message for parallel queries as an experiment. It can be removed, if you re not convinced.

---
Benoit Lobréau
Consultant
http://dalibo.com
From 7e63f79226357f2fe84acedb950e64383e582b17 Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobr...@dalibo.com>
Date: Tue, 8 Oct 2024 12:39:41 +0200
Subject: [PATCH 1/5] Add a guc for parallel worker logging

The new guc log_parallel_workers controls whether a log message is
produced to display information on the number of workers spawned when a
parallel query or utility is executed.

The default value is `none` which disables logging. `all` displays
information for all parallel queries, whereas `failures` displays
information only when the number of workers launched is lower than the
number of planned workers.

This new parameter can help database administrators and developers
diagnose performance issues related to parallelism and optimize the
configuration of the system accordingly.
---
 doc/src/sgml/config.sgml                      | 18 ++++++++++++++++++
 src/backend/access/transam/parallel.c         | 15 +++++++++++++++
 src/backend/utils/misc/guc_tables.c           | 12 ++++++++++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/access/parallel.h                 | 19 +++++++++++++++++++
 5 files changed, 65 insertions(+)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index fbdd6ce574..3a5f7131b3 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7752,6 +7752,24 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
       </listitem>
      </varlistentry>
 
+    <varlistentry id="guc-log-parallel-workers" xreflabel="log_parallel_workers">
+      <term><varname>log_parallel_workers</varname> (<type>enum</type>)
+      <indexterm>
+       <primary><varname>log_parallel_workers</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Controls whether a log message about the number of workers is produced during the
+        execution of a parallel query or utility statement. The default value is
+        <literal>none</literal> which disables logging. <literal>all</literal> displays
+        information for all parallel queries, whereas <literal>failures</literal> displays
+        information only when the number of workers launched is lower than the number of
+        planned workers.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-log-parameter-max-length" xreflabel="log_parameter_max_length">
       <term><varname>log_parameter_max_length</varname> (<type>integer</type>)
       <indexterm>
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 0a1e089ec1..e1d378efa6 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1653,3 +1653,18 @@ LookupParallelWorkerFunction(const char *libraryname, const char *funcname)
 	return (parallel_worker_main_type)
 		load_external_function(libraryname, funcname, true, NULL);
 }
+
+/*
+ * The function determines if information about workers should
+ * be logged.
+*/
+bool
+LoggingParallelWorkers(int log_parallel_workers,
+					   int parallel_workers_to_launch,
+					   int parallel_workers_launched)
+{
+	return ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&
+			parallel_workers_to_launch > 0) ||
+			(log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&
+			parallel_workers_to_launch != parallel_workers_launched));
+}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8cf1afbad2..cc4d70177d 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -482,6 +482,7 @@ extern const struct config_enum_entry archive_mode_options[];
 extern const struct config_enum_entry recovery_target_action_options[];
 extern const struct config_enum_entry wal_sync_method_options[];
 extern const struct config_enum_entry dynamic_shared_memory_options[];
+extern const struct config_enum_entry log_parallel_workers_options[];
 
 /*
  * GUC option variables that are exported from this module
@@ -526,6 +527,7 @@ int			log_min_duration_statement = -1;
 int			log_parameter_max_length = -1;
 int			log_parameter_max_length_on_error = 0;
 int			log_temp_files = -1;
+int			log_parallel_workers = LOG_PARALLEL_WORKERS_NONE;
 double		log_statement_sample_rate = 1.0;
 double		log_xact_sample_rate = 0;
 char	   *backtrace_functions;
@@ -5226,6 +5228,16 @@ struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"log_parallel_workers", PGC_SUSET, LOGGING_WHAT,
+			gettext_noop("Log information about parallel worker usage"),
+			NULL
+		},
+		&log_parallel_workers,
+		LOG_PARALLEL_WORKERS_NONE, log_parallel_workers_options,
+		NULL, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, 0, NULL, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index a2ac7575ca..f11bf173ff 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -622,6 +622,7 @@
 #log_temp_files = -1			# log temporary files equal or larger
 					# than the specified size in kilobytes;
 					# -1 disables, 0 logs all temp files
+#log_parallel_workers = none            # none, all, failure
 #log_timezone = 'GMT'
 
 # - Process Title -
diff --git a/src/include/access/parallel.h b/src/include/access/parallel.h
index 69ffe5498f..00aad1e742 100644
--- a/src/include/access/parallel.h
+++ b/src/include/access/parallel.h
@@ -19,6 +19,20 @@
 #include "postmaster/bgworker.h"
 #include "storage/shm_mq.h"
 #include "storage/shm_toc.h"
+#include "utils/guc.h"
+
+typedef enum log_parallel_workers_option_list {
+	LOG_PARALLEL_WORKERS_NONE=0,
+	LOG_PARALLEL_WORKERS_ALL,
+	LOG_PARALLEL_WORKERS_FAILURE,
+} log_parallel_workers_option_list;
+
+static const struct config_enum_entry log_parallel_workers_options[] = {
+	{"none", LOG_PARALLEL_WORKERS_NONE, false},
+	{"all", LOG_PARALLEL_WORKERS_ALL, false},
+	{"failure", LOG_PARALLEL_WORKERS_FAILURE, false},
+	{NULL, 0, false}
+};
 
 typedef void (*parallel_worker_main_type) (dsm_segment *seg, shm_toc *toc);
 
@@ -56,6 +70,7 @@ typedef struct ParallelWorkerContext
 extern PGDLLIMPORT volatile sig_atomic_t ParallelMessagePending;
 extern PGDLLIMPORT int ParallelWorkerNumber;
 extern PGDLLIMPORT bool InitializingParallelWorker;
+extern PGDLLEXPORT int log_parallel_workers;
 
 #define		IsParallelWorker()		(ParallelWorkerNumber >= 0)
 
@@ -78,4 +93,8 @@ extern void ParallelWorkerReportLastRecEnd(XLogRecPtr last_xlog_end);
 
 extern void ParallelWorkerMain(Datum main_arg);
 
+extern bool LoggingParallelWorkers(int log_parallel_workers,
+								   int parallel_workers_to_launch,
+								   int parallel_workers_launched);
+
 #endif							/* PARALLEL_H */
-- 
2.47.0

From 2894c62d8f550d6030d1327b322864805aaa8c5f Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobr...@dalibo.com>
Date: Tue, 8 Oct 2024 12:45:03 +0200
Subject: [PATCH 2/5] Implements logging for parallel worker usage in index
 builds

Two types of index are concerned: brin and btree.
---
 src/backend/access/brin/brin.c      | 7 +++++++
 src/backend/access/nbtree/nbtsort.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 9af445cdcd..a19e1eed7f 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -2544,6 +2544,13 @@ _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state)
 	/* Shutdown worker processes */
 	WaitForParallelWorkersToFinish(brinleader->pcxt);
 
+	if (LoggingParallelWorkers(log_parallel_workers,
+							   brinleader->pcxt->nworkers_to_launch,
+							   brinleader->pcxt->nworkers_launched))
+		elog(LOG, "%i workers planned (%i workers launched)",
+			brinleader->pcxt->nworkers_to_launch,
+			brinleader->pcxt->nworkers_launched);
+
 	/*
 	 * Next, accumulate WAL usage.  (This must wait for the workers to finish,
 	 * or we might get incomplete data.)
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 28522c0ac1..6459a01618 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -1614,6 +1614,13 @@ _bt_end_parallel(BTLeader *btleader)
 	/* Shutdown worker processes */
 	WaitForParallelWorkersToFinish(btleader->pcxt);
 
+	if (LoggingParallelWorkers(log_parallel_workers,
+							   btleader->pcxt->nworkers_to_launch,
+							   btleader->pcxt->nworkers_launched))
+		elog(LOG, "%i workers planned (%i workers launched)",
+			btleader->pcxt->nworkers_to_launch,
+			btleader->pcxt->nworkers_launched);
+
 	/*
 	 * Next, accumulate WAL usage.  (This must wait for the workers to finish,
 	 * or we might get incomplete data.)
-- 
2.47.0

From 477b08125e4131b2199ad7c38560c6ce3a0f63fd Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobr...@dalibo.com>
Date: Fri, 11 Oct 2024 23:56:23 +0200
Subject: [PATCH 3/5] Setup counters for parallel vacuums

Thsi can be used bu other patches such as the one for pg_stat_database.
---
 src/backend/commands/vacuumparallel.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 67cba17a56..7094c80dc6 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -208,6 +208,9 @@ struct ParallelVacuumState
 	int			nindexes_parallel_cleanup;
 	int			nindexes_parallel_condcleanup;
 
+	int			nworkers_to_launch;
+	int			nworkers_launched;
+
 	/* Buffer access strategy used by leader process */
 	BufferAccessStrategy bstrategy;
 
@@ -362,6 +365,9 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes,
 		if ((vacoptions & VACUUM_OPTION_PARALLEL_COND_CLEANUP) != 0)
 			pvs->nindexes_parallel_condcleanup++;
 	}
+	pvs->nworkers_to_launch = 0;
+	pvs->nworkers_launched = 0;
+
 	shm_toc_insert(pcxt->toc, PARALLEL_VACUUM_KEY_INDEX_STATS, indstats);
 	pvs->indstats = indstats;
 
@@ -738,6 +744,9 @@ parallel_vacuum_process_all_indexes(ParallelVacuumState *pvs, int num_index_scan
 
 		for (int i = 0; i < pvs->pcxt->nworkers_launched; i++)
 			InstrAccumParallelQuery(&pvs->buffer_usage[i], &pvs->wal_usage[i]);
+
+		pvs->nworkers_to_launch += pvs->pcxt->nworkers_to_launch;
+		pvs->nworkers_launched += pvs->pcxt->nworkers_launched;
 	}
 
 	/*
-- 
2.47.0

From a3c12d9991e6063025979c8fb3f90a256f8b42e0 Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobr...@dalibo.com>
Date: Fri, 11 Oct 2024 23:58:40 +0200
Subject: [PATCH 4/5] Implements logging for parallel worker usage in vacuums

---
 src/backend/commands/vacuumparallel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 7094c80dc6..efce9c155f 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -443,6 +443,13 @@ parallel_vacuum_end(ParallelVacuumState *pvs, IndexBulkDeleteResult **istats)
 {
 	Assert(!IsParallelWorker());
 
+	if (LoggingParallelWorkers(log_parallel_workers,
+							   pvs->nworkers_to_launch,
+							   pvs->nworkers_launched))
+		elog(LOG, "%i workers planned to be launched for index cleanup and bulkdelete (%i workers launched)",
+			pvs->nworkers_to_launch,
+			pvs->nworkers_launched);
+
 	/* Copy the updated statistics */
 	for (int i = 0; i < pvs->nindexes; i++)
 	{
-- 
2.47.0

From 4886538cb5d2542b36af6793b8827d695a44ab5b Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobr...@dalibo.com>
Date: Fri, 11 Oct 2024 23:59:15 +0200
Subject: [PATCH 5/5] Implements logging for parallel worker usage in queries

---
 src/backend/executor/execMain.c        | 11 +++++++++++
 src/backend/executor/execUtils.c       |  3 +++
 src/backend/executor/nodeGather.c      |  5 +++++
 src/backend/executor/nodeGatherMerge.c |  5 +++++
 src/include/nodes/execnodes.h          |  4 ++++
 5 files changed, 28 insertions(+)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 1c12d6ebff..81da8f5e17 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -485,6 +485,17 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
 		pgstat_update_parallel_workers_stats((PgStat_Counter) estate->es_parallel_workers_to_launch,
 											 (PgStat_Counter) estate->es_parallel_workers_launched);
 
+	if (LoggingParallelWorkers(log_parallel_workers,
+							   estate->es_parallel_workers_to_launch,
+							   estate->es_parallel_workers_launched))
+		elog(LOG, "%i parallel nodes planned (%i obtained all their workers, %i obtained none), "
+				  "%i workers planned to be launched (%i workers launched)",
+			estate->es_parallel_nodes,
+			estate->es_parallel_nodes_success,
+			estate->es_parallel_nodes_no_workers,
+			estate->es_parallel_workers_to_launch,
+			estate->es_parallel_workers_launched);
+
 	/*
 	 * Check that ExecutorFinish was called, unless in EXPLAIN-only mode. This
 	 * Assert is needed because ExecutorFinish is new as of 9.1, and callers
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index df0223129c..c38253f5e5 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -160,6 +160,9 @@ CreateExecutorState(void)
 	estate->es_use_parallel_mode = false;
 	estate->es_parallel_workers_to_launch = 0;
 	estate->es_parallel_workers_launched = 0;
+	estate->es_parallel_nodes = 0;
+	estate->es_parallel_nodes_success = 0;
+	estate->es_parallel_nodes_no_workers = 0;
 
 	estate->es_jit_flags = 0;
 	estate->es_jit = NULL;
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index 7f7edc7f9f..49a96f7cbf 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -188,6 +188,10 @@ ExecGather(PlanState *pstate)
 			 */
 			estate->es_parallel_workers_to_launch += pcxt->nworkers_to_launch;
 			estate->es_parallel_workers_launched += pcxt->nworkers_launched;
+			estate->es_parallel_nodes += 1;
+
+			if (pcxt->nworkers_to_launch == pcxt->nworkers_launched)
+				estate->es_parallel_nodes_success += 1;
 
 			/* Set up tuple queue readers to read the results. */
 			if (pcxt->nworkers_launched > 0)
@@ -205,6 +209,7 @@ ExecGather(PlanState *pstate)
 				/* No workers?	Then never mind. */
 				node->nreaders = 0;
 				node->reader = NULL;
+				estate->es_parallel_nodes_no_workers += 1;
 			}
 			node->nextreader = 0;
 		}
diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c
index bc99c0b448..f3aa7ee14f 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -229,6 +229,10 @@ ExecGatherMerge(PlanState *pstate)
 			 */
 			estate->es_parallel_workers_to_launch += pcxt->nworkers_to_launch;
 			estate->es_parallel_workers_launched += pcxt->nworkers_launched;
+			estate->es_parallel_nodes += 1;
+
+			if (pcxt->nworkers_to_launch == pcxt->nworkers_launched)
+				estate->es_parallel_nodes_success += 1;
 
 			/* Set up tuple queue readers to read the results. */
 			if (pcxt->nworkers_launched > 0)
@@ -246,6 +250,7 @@ ExecGatherMerge(PlanState *pstate)
 				/* No workers?	Then never mind. */
 				node->nreaders = 0;
 				node->reader = NULL;
+				estate->es_parallel_nodes_no_workers += 1;
 			}
 		}
 
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 1590b64392..dd1fc5a18b 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -713,6 +713,10 @@ typedef struct EState
 	int			es_parallel_workers_launched;	/* number of workers actually
 												 * launched. */
 
+	int			es_parallel_nodes;
+	int			es_parallel_nodes_success;
+	int			es_parallel_nodes_no_workers;
+
 	/* The per-query shared memory area to use for parallel execution. */
 	struct dsa_area *es_query_dsa;
 
-- 
2.47.0

Reply via email to