From: Alena Rybakina <a.rybakina@postgrespro.ru>
To: Benoit Lobréau <benoit.lobreau@dalibo.com>
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: Mon, 14 Oct 2024 23:42:09 +0300
Message-ID: <c14381b1-1f7a-4677-8b94-baa4dead9bc5@postgrespro.ru> (raw)
In-Reply-To: <d4825c9e-9fd0-440e-aa3a-411f0b92bfc9@postgrespro.ru>
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>
Sorry, I was in a hurry and didn't check my patch properly.
On 14.10.2024 23:20, Alena Rybakina wrote:
> On 28.08.2024 15:58, Benoit Lobréau wrote:
>> Hi,
>>
>> Here is a new version of the patch. Sorry for the long delay, I was
>> hit by a motivation drought and was quite busy otherwise.
>>
>> The guc is now called `log_parallel_workers` and has three possible
>> values:
>>
>> * "none": disables logging
>> * "all": logs parallel worker info for all parallel queries or utilities
>> * "failure": logs only when the number of parallel workers planned
>> couldn't be reached.
>>
>> For this, I added several members to the EState struct.
>>
>> Each gather node / gather merge node updates the values and the
>> offending queries are displayed during standard_ExecutorEnd.
>>
>> For CREATE INDEX / REINDEX on btree and brin, I check the parallel
>> context struct (pcxt) during _bt_end_parallel() or
>> _brin_end_parallel() and display a log message when needed.
>>
>> For vacuum, I do the same in parallel_vacuum_end().
>>
>> I added some information to the error message for parallel queries as
>> an experiment. I find it useful, but it can be removed, if you re not
>> convinced.
>>
>> 2024-08-27 15:59:11.089 CEST [54585] LOG: 1 parallel nodes planned
>> (1 obtained all their workers, 0 obtained none), 2 workers planned (2
>> workers launched)
>> 2024-08-27 15:59:11.089 CEST [54585] STATEMENT: EXPLAIN (ANALYZE)
>> SELECT i, avg(j) FROM test_pql GROUP BY i;
>>
>> 2024-08-27 15:59:14.006 CEST [54585] LOG: 2 parallel nodes planned
>> (0 obtained all their workers, 1 obtained none), 4 workers planned (1
>> workers launched)
>> 2024-08-27 15:59:14.006 CEST [54585] STATEMENT: EXPLAIN (ANALYZE)
>> SELECT i, avg(j) FROM test_pql GROUP BY i
>> UNION
>> SELECT i, avg(j) FROM test_pql GROUP BY i;
>>
>> For CREATE INDEX / REDINDEX / VACUUMS:
>>
>> 2024-08-27 15:58:59.769 CEST [54521] LOG: 1 workers planned (0
>> workers launched)
>> 2024-08-27 15:58:59.769 CEST [54521] STATEMENT: REINDEX TABLE test_pql;
>>
>> Do you think this is better?
>>
>> I am not sure if a struct is needed to store the es_nworkers* and if
>> the modification I did to parallel.h is ok.
>>
>> Thanks to: Jehan-Guillaume de Rorthais, Guillaume Lelarge and Franck
>> Boudehen for the help and motivation boost.
>
> Hi!
>
> I think it's a good idea to log this. I suggest some changes that
> might improve your patch.
>
>
> 2. I think you can transfer nworkers_to_launch and nworkers_launched
> vacuum parameters in the ParallelContext struct.
Sorry, I didn’t notice right away that the second point is impossible to
fulfill here.
Attached is the correct version of the patch.
--
Regards,
Alena Rybakina
Postgres Professional
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgmlindex 5c04a27eb66..1e2fadffaf1 100644--- a/doc/src/sgml/config.sgml+++ b/doc/src/sgml/config.sgml@@ -7704,8 +7704,8 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
</term>
<listitem>
<para>
- 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+ Controls whether a log message about the number of workers produced during an+ execution of the 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
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.cindex fdfbbc7dc42..402be20e27c 100644--- a/src/backend/access/brin/brin.c+++ b/src/backend/access/brin/brin.c@@ -2544,10 +2544,9 @@ _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state)
/* Shutdown worker processes */
WaitForParallelWorkersToFinish(brinleader->pcxt);
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- brinleader->pcxt->nworkers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- brinleader->pcxt->nworkers_to_launch > brinleader->pcxt->nworkers_launched))+ 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);
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.cindex e31bd8e223e..1ca027d2727 100644--- a/src/backend/access/nbtree/nbtsort.c+++ b/src/backend/access/nbtree/nbtsort.c@@ -1615,10 +1615,9 @@ _bt_end_parallel(BTLeader *btleader)
/* Shutdown worker processes */
WaitForParallelWorkersToFinish(btleader->pcxt);
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- btleader->pcxt->nworkers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- btleader->pcxt->nworkers_to_launch > btleader->pcxt->nworkers_launched))+ 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);
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.cindex d4e84aabac7..8a0d5028b1c 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 of logging information+ * about workers.+*/+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));+}
\ No newline at end of file
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.cindex e207a6b7d91..84b0a90921c 100644--- a/src/backend/commands/vacuumparallel.c+++ b/src/backend/commands/vacuumparallel.c@@ -443,10 +443,9 @@ parallel_vacuum_end(ParallelVacuumState *pvs, IndexBulkDeleteResult **istats)
{
Assert(!IsParallelWorker());
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- pvs->nworkers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- pvs->nworkers_to_launch > pvs->nworkers_launched))+ 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);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.cindex 6663fea7b12..b85d679b7fd 100644--- a/src/backend/executor/execMain.c+++ b/src/backend/executor/execMain.c@@ -483,10 +483,9 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
Assert(estate != NULL);
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- estate->es_parallel_workers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- estate->es_parallel_workers_to_launch != 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,
diff --git a/src/include/access/parallel.h b/src/include/access/parallel.hindex ed33e545d76..00aad1e742a 100644--- a/src/include/access/parallel.h+++ b/src/include/access/parallel.h@@ -93,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 */
Attachments:
[text/plain] parallel_log_workers.no-cfbot (5.7K, ../c14381b1-1f7a-4677-8b94-baa4dead9bc5@postgrespro.ru/2-parallel_log_workers.no-cfbot)
download | inline diff:diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgmlindex 5c04a27eb66..1e2fadffaf1 100644--- a/doc/src/sgml/config.sgml+++ b/doc/src/sgml/config.sgml@@ -7704,8 +7704,8 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
</term>
<listitem>
<para>
- 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+ Controls whether a log message about the number of workers produced during an+ execution of the 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
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.cindex fdfbbc7dc42..402be20e27c 100644--- a/src/backend/access/brin/brin.c+++ b/src/backend/access/brin/brin.c@@ -2544,10 +2544,9 @@ _brin_end_parallel(BrinLeader *brinleader, BrinBuildState *state)
/* Shutdown worker processes */
WaitForParallelWorkersToFinish(brinleader->pcxt);
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- brinleader->pcxt->nworkers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- brinleader->pcxt->nworkers_to_launch > brinleader->pcxt->nworkers_launched))+ 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);
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.cindex e31bd8e223e..1ca027d2727 100644--- a/src/backend/access/nbtree/nbtsort.c+++ b/src/backend/access/nbtree/nbtsort.c@@ -1615,10 +1615,9 @@ _bt_end_parallel(BTLeader *btleader)
/* Shutdown worker processes */
WaitForParallelWorkersToFinish(btleader->pcxt);
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- btleader->pcxt->nworkers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- btleader->pcxt->nworkers_to_launch > btleader->pcxt->nworkers_launched))+ 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);
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.cindex d4e84aabac7..8a0d5028b1c 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 of logging information+ * about workers.+*/+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));+}
\ No newline at end of file
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.cindex e207a6b7d91..84b0a90921c 100644--- a/src/backend/commands/vacuumparallel.c+++ b/src/backend/commands/vacuumparallel.c@@ -443,10 +443,9 @@ parallel_vacuum_end(ParallelVacuumState *pvs, IndexBulkDeleteResult **istats)
{
Assert(!IsParallelWorker());
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- pvs->nworkers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- pvs->nworkers_to_launch > pvs->nworkers_launched))+ 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);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.cindex 6663fea7b12..b85d679b7fd 100644--- a/src/backend/executor/execMain.c+++ b/src/backend/executor/execMain.c@@ -483,10 +483,9 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
Assert(estate != NULL);
- if ((log_parallel_workers == LOG_PARALLEL_WORKERS_ALL &&- estate->es_parallel_workers_to_launch > 0) ||- (log_parallel_workers == LOG_PARALLEL_WORKERS_FAILURE &&- estate->es_parallel_workers_to_launch != 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,
diff --git a/src/include/access/parallel.h b/src/include/access/parallel.hindex ed33e545d76..00aad1e742a 100644--- a/src/include/access/parallel.h+++ b/src/include/access/parallel.h@@ -93,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 */
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: a.rybakina@postgrespro.ru, benoit.lobreau@dalibo.com, 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: <c14381b1-1f7a-4677-8b94-baa4dead9bc5@postgrespro.ru>
* 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