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.sgml
index 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.c
index 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.c
index 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.c
index 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.c
index 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.c
index 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.h
index 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 */
