pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Benoit Lobréau <benoit.lobreau@dalibo.com>
To: Alena Rybakina <a.rybakina@postgrespro.ru>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Imseih (AWS), Sami <simseih@amazon.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>
Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>
Subject: Re: Logging parallel worker draught
Date: Sat, 19 Oct 2024 06:46:44 +0200
Message-ID: <fd9b82bc-c853-4143-ba8f-35a693ee439c@dalibo.com> (raw )
In-Reply-To: <63d182f8-1a65-4e34-a335-403f53b14ba7@dalibo.com >
References: <10D5C31A-A303-4440-BCA2-921440A9E1CC@amazon.com >
<202310111011.lyo66rk7k7pu@alvherre.pgsql >
<9E9E69BD-BABB-49BB-8B69-61939179F20D@amazon.com >
<11e34b80-b0a6-e2e4-1606-1f5077379a34@dalibo.com >
<D04977E3-9F54-452C-A4C4-CDA67F392BD1@amazon.com >
<0d08b978-10cb-4512-bcef-ca1f192049ab@enterprisedb.com >
<0b7ef57e-bf30-440b-9478-d8f587f2637c@dalibo.com >
<2cccd814-d7b3-4c9a-a66a-6df4c45fd95e@enterprisedb.com >
<17f9faf1-4edb-46d0-80ed-91bba16423a3@dalibo.com >
<7ECC97AE-2670-4792-BC6F-FD91B23F8E8C@yandex-team.ru >
<39ac8951-6b99-469b-b9f1-2791efe43fbd@dalibo.com >
<8123423a-f041-4f4c-a771-bfd96ab235b0@dalibo.com >
<2a8e9e1c-0f05-4ebd-9a4d-d023d26c0def@dalibo.com >
<04e6d7e2-f67a-438c-bc11-9354ab64ef1f@dalibo.com >
<fbf19680-ce74-4f27-a890-e427ab330411@dalibo.com >
<d4825c9e-9fd0-440e-aa3a-411f0b92bfc9@postgrespro.ru >
<c14381b1-1f7a-4677-8b94-baa4dead9bc5@postgrespro.ru >
<63d182f8-1a65-4e34-a335-403f53b14ba7@dalibo.com >
On 10/15/24 09:52, Benoit Lobréau wrote:
> Thank you, it's a lot cleaner that way.
> I'll add this asap.
This is an updated version with the suggested changes.
--
Benoit Lobréau
Consultant
http://dalibo.com
Attachments: download all (5 files, .tar.gz)
V4_0005-Implements-logging-for-parallel-worker-usage-in-quer.patch (text/x-patch, 4.2K) download
From b9bf7c0fa967c72972fd77333a768dfe89d91765 Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobreau@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 cc9a594cba..b85d679b7f 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -483,6 +483,17 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
Assert(estate != NULL);
+ 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 6712302ec8..fd463f7e4c 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 e4698a28c4..d87af53853 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.46.2
V4_0004-Implements-logging-for-parallel-worker-usage-in-vacu.patch (text/x-patch, 1.0K) download
From f13f1da31e4e45c68c3afae5c71572008caf6e84 Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobreau@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 77679e8df6..84b0a90921 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.46.2
V4_0003-Setup-counters-for-parallel-vacuums.patch (text/x-patch, 1.6K) download
From 9106db2f93d2d4087dcbe6e2da74043edc535ae5 Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobreau@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 4fd6574e12..77679e8df6 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;
@@ -739,6 +745,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.46.2
V4_0002-Implements-logging-for-parallel-worker-usage-in-inde.patch (text/x-patch, 1.9K) download
From e34c1f3634adf1f0fd927c303e90c4ef873884ce Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobreau@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 c0b978119a..402be20e27 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 5cca0d4f52..1ca027d272 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -1615,6 +1615,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.46.2
V4_0001-Add-a-guc-for-parallel-worker-logging.patch (text/x-patch, 6.6K) download
From 907f83e81031c8faa839acdef5d07e57f046ade1 Mon Sep 17 00:00:00 2001
From: benoit <benoit.lobreau@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 934ef5e469..541b1a0f6e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7696,6 +7696,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 d4e84aabac..cfd73dc64e 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1627,3 +1627,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 2c4cc8cd41..94b6a237b1 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;
@@ -5196,6 +5198,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 667e0dc40a..7cd9c0f3f8 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -619,6 +619,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.46.2
view thread (53+ messages) latest in thread
Message-ID: <fd9b82bc-c853-4143-ba8f-35a693ee439c@dalibo.com>
Permalink: ../fd9b82bc-c853-4143-ba8f-35a693ee439c@dalibo.com/
Also on: postgresql.org/message-id/fd9b82bc-c853-4143-ba8f-35a693ee439c@dalibo.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: benoit.lobreau@dalibo.com, a.rybakina@postgrespro.ru, simseih@amazon.com, alvherre@alvh.no-ip.org, robertmhaas@gmail.com, amit.kapila16@gmail.com, melanieplageman@gmail.com, x4mmm@yandex-team.ru
Subject: Re: Logging parallel worker draught
In-Reply-To: <fd9b82bc-c853-4143-ba8f-35a693ee439c@dalibo.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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