On Thu, Apr 08, 2021 at 05:46:07PM +0530, Amit Kapila wrote:
>
> @@ -1421,8 +1421,9 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc)
> /* Setting debug_query_string for individual workers */
> debug_query_string = queryDesc->sourceText;
>
> - /* Report workers' query for monitoring purposes */
> + /* Report workers' query and queryId for monitoring purposes */
> pgstat_report_activity(STATE_RUNNING, debug_query_string);
> + pgstat_report_queryid(queryDesc->plannedstmt->queryId, false);
>
>
> Below lines down in ParallelQueryMain, we call ExecutorStart which
> will report queryid, so do we need it here?
Correct, it's not actually needed. The overhead should be negligible but let's
get rid of it. Updated fix patchset attached.
>From d74523dfb76e7583c27166ec10d72670654c3b7b Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouh...@free.fr>
Date: Thu, 8 Apr 2021 13:59:43 +0800
Subject: [PATCH v3 1/3] Ignore parallel workers in pg_stat_statements.
Oversight in 4f0b0966c8 which exposed queryid in parallel workers. Counters
are aggregated by the main backend process so parallel workers would report
duplicated activity, and could also report activity for the wrong entry as they
are only aware of the top level queryid.
Author: Julien Rouhaud
Reported-by: Andres Freund
Discussion: https://postgr.es/m/20210408051735.lfbdzun5zdlax...@alap3.anarazel.de
---
contrib/pg_stat_statements/pg_stat_statements.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index fc2677643b..dbd0d41d88 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -47,6 +47,7 @@
#include <sys/stat.h>
#include <unistd.h>
+#include "access/parallel.h"
#include "catalog/pg_authid.h"
#include "common/hashfn.h"
#include "executor/instrument.h"
@@ -278,8 +279,9 @@ static bool pgss_save; /* whether to save stats across shutdown */
#define pgss_enabled(level) \
+ (!IsParallelWorker() && \
(pgss_track == PGSS_TRACK_ALL || \
- (pgss_track == PGSS_TRACK_TOP && (level) == 0))
+ (pgss_track == PGSS_TRACK_TOP && (level) == 0)))
#define record_gc_qtexts() \
do { \
--
2.30.1
>From 61ff6d226761fcc8f2a28fe8e313382d1d46f098 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouh...@free.fr>
Date: Thu, 8 Apr 2021 20:05:14 +0800
Subject: [PATCH v3 2/3] Fix thinko in pg_stat_get_activity when retrieving the
queryid.
---
src/backend/utils/adt/pgstatfuncs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9fa4a93162..182b15e3f2 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -917,7 +917,7 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
if (beentry->st_queryid == 0)
nulls[29] = true;
else
- values[29] = DatumGetUInt64(beentry->st_queryid);
+ values[29] = UInt64GetDatum(beentry->st_queryid);
}
else
{
--
2.30.1
>From bc3b5470d12aab282e1b6f6b0b2f4afcc65b7dcf Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouh...@free.fr>
Date: Thu, 8 Apr 2021 20:25:19 +0800
Subject: [PATCH v3 3/3] Remove unnecessary call to pgstat_report_queryid().
Reported-by: Amit Kapila
---
src/backend/executor/execParallel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index d104a19767..bfd6155509 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -1426,7 +1426,7 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc)
/* Report workers' query and queryId for monitoring purposes */
pgstat_report_activity(STATE_RUNNING, debug_query_string);
- pgstat_report_queryid(queryDesc->plannedstmt->queryId, false);
+ //pgstat_report_queryid(queryDesc->plannedstmt->queryId, false);
/* Attach to the dynamic shared memory area. */
area_space = shm_toc_lookup(toc, PARALLEL_KEY_DSA, false);
--
2.30.1
