Hi,

Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <bertranddrouvot...@gmail.com>
a écrit :

> Hi,
>
> On Wed, Sep 04, 2024 at 02:51:51PM +0200, Guillaume Lelarge wrote:
> > Hi,
> >
> > Le mer. 4 sept. 2024 à 10:47, Bertrand Drouvot <
> bertranddrouvot...@gmail.com>
> > a écrit :
> >
> > > What about to get rid of the pgstat_count_parallel_heap_scan and add an
> > > extra
> > > bolean parameter to pgstat_count_heap_scan to indicate if
> > > counts.parallelnumscans
> > > should be incremented too?
> > >
> > > Something like:
> > >
> > > pgstat_count_heap_scan(scan->rs_base.rs_rd, scan->rs_base.rs_parallel
> !=
> > > NULL)
> > >
> > > 3 ===
> > >
> > > Same comment for pgstat_count_index_scan (add an extra bolean
> parameter)
> > > and
> > > get rid of pgstat_count_parallel_index_scan()).
> > >
> > > I think that 2 === and 3 === would help to avoid missing increments
> should
> > > we
> > > add those call to other places in the future.
> > >
> > >
> > Oh OK, understood.  Done for both.
>
> Thanks for v2!
>
> 1 ===
>
> -#define pgstat_count_heap_scan(rel)
> +#define pgstat_count_heap_scan(rel, parallel)
>         do {
> -               if (pgstat_should_count_relation(rel))
> -                       (rel)->pgstat_info->counts.numscans++;
> +            if (pgstat_should_count_relation(rel)) {
> +                       if (!parallel)
> +                               (rel)->pgstat_info->counts.numscans++;
> +                       else
> +
>  (rel)->pgstat_info->counts.parallelnumscans++;
> +               }
>
> I think counts.numscans has to be incremented in all the cases (so even if
> "parallel" is true).
>
> Same comment for pgstat_count_index_scan().
>
>
You're right, and I've been too quick. Fixed in v3.


> > 4 ===
> > >
> > > +       if (lstats->counts.numscans || lstats->counts.parallelnumscans)
> > >
> > > Is it possible to have (lstats->counts.parallelnumscans) whithout
> having
> > > (lstats->counts.numscans) ?
> > >
> > >
> > Nope, parallel scans are included in seq/index scans, as far as I can
> tell.
> > I could remove the parallelnumscans testing but it would be less obvious
> to
> > read.
>
> 2 ===
>
> What about adding a comment instead of this extra check?
>
>
Done too in v3.


-- 
Guillaume.
From 97a95650cd220c1b88ab6f3d36149b8860bead1d Mon Sep 17 00:00:00 2001
From: Guillaume Lelarge <guillaume.lela...@dalibo.com>
Date: Wed, 28 Aug 2024 21:35:30 +0200
Subject: [PATCH v3] Add parallel columns for pg_stat_all_tables,indexes

pg_stat_all_tables gets 4 new columns: parallel_seq_scan,
last_parallel_seq_scan, parallel_idx_scan, last_parallel_idx_scan.

pg_stat_all_indexes gets 2 new columns: parallel_idx_scan,
last_parallel_idx_scan.
---
 doc/src/sgml/monitoring.sgml                 | 69 ++++++++++++++++++--
 src/backend/access/brin/brin.c               |  2 +-
 src/backend/access/gin/ginscan.c             |  2 +-
 src/backend/access/gist/gistget.c            |  4 +-
 src/backend/access/hash/hashsearch.c         |  2 +-
 src/backend/access/heap/heapam.c             |  2 +-
 src/backend/access/nbtree/nbtsearch.c        |  2 +-
 src/backend/access/spgist/spgscan.c          |  2 +-
 src/backend/catalog/system_views.sql         |  6 ++
 src/backend/utils/activity/pgstat_relation.c | 10 ++-
 src/backend/utils/adt/pgstatfuncs.c          |  6 ++
 src/include/catalog/pg_proc.dat              |  8 +++
 src/include/pgstat.h                         | 17 +++--
 13 files changed, 113 insertions(+), 19 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 933de6fe07..6886094095 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3773,7 +3773,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <structfield>seq_scan</structfield> <type>bigint</type>
       </para>
       <para>
-       Number of sequential scans initiated on this table
+       Number of sequential scans (including parallel ones) initiated on this table
       </para></entry>
      </row>
 
@@ -3782,7 +3782,26 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <structfield>last_seq_scan</structfield> <type>timestamp with time zone</type>
       </para>
       <para>
-       The time of the last sequential scan on this table, based on the
+       The time of the last sequential scan (including parallel ones) on this table, based on the
+       most recent transaction stop time
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>parallel_seq_scan</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of parallel sequential scans initiated on this table
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>last_parallel_seq_scan</structfield> <type>timestamp with time zone</type>
+      </para>
+      <para>
+       The time of the last parallel sequential scan on this table, based on the
        most recent transaction stop time
       </para></entry>
      </row>
@@ -3801,7 +3820,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <structfield>idx_scan</structfield> <type>bigint</type>
       </para>
       <para>
-       Number of index scans initiated on this table
+       Number of index scans (including parallel ones) initiated on this table
       </para></entry>
      </row>
 
@@ -3810,7 +3829,26 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <structfield>last_idx_scan</structfield> <type>timestamp with time zone</type>
       </para>
       <para>
-       The time of the last index scan on this table, based on the
+       The time of the last index scan (including parallel ones) on this table, based on the
+       most recent transaction stop time
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>parallel_idx_scan</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of parallel index scans initiated on this table
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>last_parallel_idx_scan</structfield> <type>timestamp with time zone</type>
+      </para>
+      <para>
+       The time of the last parallel index scan on this table, based on the
        most recent transaction stop time
       </para></entry>
      </row>
@@ -4080,7 +4118,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <structfield>idx_scan</structfield> <type>bigint</type>
       </para>
       <para>
-       Number of index scans initiated on this index
+       Number of index scans (including parallel ones) initiated on this index
       </para></entry>
      </row>
 
@@ -4089,7 +4127,26 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <structfield>last_idx_scan</structfield> <type>timestamp with time zone</type>
       </para>
       <para>
-       The time of the last scan on this index, based on the
+       The time of the last scan on this index(including parallel ones), based
+       on the most recent transaction stop time
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>parallel_idx_scan</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of parallel index scans initiated on this index
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>last_parallel_idx_scan</structfield> <type>timestamp with time zone</type>
+      </para>
+      <para>
+       The time of the last parallel scan on this index, based on the
        most recent transaction stop time
       </para></entry>
      </row>
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 6467bed604..4b5557fcf7 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -580,7 +580,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 
 	opaque = (BrinOpaque *) scan->opaque;
 	bdesc = opaque->bo_bdesc;
-	pgstat_count_index_scan(idxRel);
+	pgstat_count_index_scan(idxRel, false);
 
 	/*
 	 * We need to know the size of the table so that we know how long to
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index af24d38544..2926a4caf6 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -435,7 +435,7 @@ ginNewScanKey(IndexScanDesc scan)
 
 	MemoryContextSwitchTo(oldCtx);
 
-	pgstat_count_index_scan(scan->indexRelation);
+	pgstat_count_index_scan(scan->indexRelation, false);
 }
 
 void
diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
index b35b8a9757..7e89382ce5 100644
--- a/src/backend/access/gist/gistget.c
+++ b/src/backend/access/gist/gistget.c
@@ -624,7 +624,7 @@ gistgettuple(IndexScanDesc scan, ScanDirection dir)
 		/* Begin the scan by processing the root page */
 		GISTSearchItem fakeItem;
 
-		pgstat_count_index_scan(scan->indexRelation);
+		pgstat_count_index_scan(scan->indexRelation, false);
 
 		so->firstCall = false;
 		so->curPageData = so->nPageData = 0;
@@ -749,7 +749,7 @@ gistgetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
 	if (!so->qual_ok)
 		return 0;
 
-	pgstat_count_index_scan(scan->indexRelation);
+	pgstat_count_index_scan(scan->indexRelation, false);
 
 	/* Begin the scan by processing the root page */
 	so->curPageData = so->nPageData = 0;
diff --git a/src/backend/access/hash/hashsearch.c b/src/backend/access/hash/hashsearch.c
index 0d99d6abc8..a63edc8372 100644
--- a/src/backend/access/hash/hashsearch.c
+++ b/src/backend/access/hash/hashsearch.c
@@ -297,7 +297,7 @@ _hash_first(IndexScanDesc scan, ScanDirection dir)
 	HashPageOpaque opaque;
 	HashScanPosItem *currItem;
 
-	pgstat_count_index_scan(rel);
+	pgstat_count_index_scan(rel, false);
 
 	/*
 	 * We do not support hash scans with no index qualification, because we
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 91b20147a0..f21cf50e6e 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -409,7 +409,7 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
 	 * and for sample scans we update stats for tuple fetches).
 	 */
 	if (scan->rs_base.rs_flags & SO_TYPE_SEQSCAN)
-		pgstat_count_heap_scan(scan->rs_base.rs_rd);
+		pgstat_count_heap_scan(scan->rs_base.rs_rd, (scan->rs_base.rs_parallel != NULL));
 }
 
 /*
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
index 2551df8a67..ef50852199 100644
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -896,7 +896,7 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
 
 	Assert(!BTScanPosIsValid(so->currPos));
 
-	pgstat_count_index_scan(rel);
+	pgstat_count_index_scan(rel, (scan->parallel_scan != NULL));
 
 	/*
 	 * Examine the scan keys and eliminate any redundant keys; also mark the
diff --git a/src/backend/access/spgist/spgscan.c b/src/backend/access/spgist/spgscan.c
index 03293a7816..a78fa34570 100644
--- a/src/backend/access/spgist/spgscan.c
+++ b/src/backend/access/spgist/spgscan.c
@@ -422,7 +422,7 @@ spgrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
 	resetSpGistScanOpaque(so);
 
 	/* count an indexscan for stats */
-	pgstat_count_index_scan(scan->indexRelation);
+	pgstat_count_index_scan(scan->indexRelation, false);
 }
 
 void
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a1..54b1cd6b40 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -670,9 +670,13 @@ CREATE VIEW pg_stat_all_tables AS
             C.relname AS relname,
             pg_stat_get_numscans(C.oid) AS seq_scan,
             pg_stat_get_lastscan(C.oid) AS last_seq_scan,
+            pg_stat_get_parallelnumscans(C.oid) AS parallel_seq_scan,
+            pg_stat_get_parallellastscan(C.oid) AS last_parallel_seq_scan,
             pg_stat_get_tuples_returned(C.oid) AS seq_tup_read,
             sum(pg_stat_get_numscans(I.indexrelid))::bigint AS idx_scan,
             max(pg_stat_get_lastscan(I.indexrelid)) AS last_idx_scan,
+            sum(pg_stat_get_parallelnumscans(I.indexrelid))::bigint AS parallel_idx_scan,
+            max(pg_stat_get_parallellastscan(I.indexrelid)) AS last_parallel_idx_scan,
             sum(pg_stat_get_tuples_fetched(I.indexrelid))::bigint +
             pg_stat_get_tuples_fetched(C.oid) AS idx_tup_fetch,
             pg_stat_get_tuples_inserted(C.oid) AS n_tup_ins,
@@ -792,6 +796,8 @@ CREATE VIEW pg_stat_all_indexes AS
             I.relname AS indexrelname,
             pg_stat_get_numscans(I.oid) AS idx_scan,
             pg_stat_get_lastscan(I.oid) AS last_idx_scan,
+            pg_stat_get_parallelnumscans(I.oid) AS parallel_idx_scan,
+            pg_stat_get_parallellastscan(I.oid) AS last_parallel_idx_scan,
             pg_stat_get_tuples_returned(I.oid) AS idx_tup_read,
             pg_stat_get_tuples_fetched(I.oid) AS idx_tup_fetch
     FROM pg_class C JOIN
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index 8a3f7d434c..b88c35d834 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -829,12 +829,20 @@ pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 	tabentry = &shtabstats->stats;
 
 	tabentry->numscans += lstats->counts.numscans;
+	tabentry->parallelnumscans += lstats->counts.parallelnumscans;
+
+	/*
+	 * Don't check counts.parallelnumscans because counts.numscans includes
+	 * counts.parallelnumscans
+	 */
 	if (lstats->counts.numscans)
 	{
 		TimestampTz t = GetCurrentTransactionStopTimestamp();
 
-		if (t > tabentry->lastscan)
+		if (t > tabentry->lastscan && lstats->counts.numscans)
 			tabentry->lastscan = t;
+		if (t > tabentry->parallellastscan && lstats->counts.parallelnumscans)
+			tabentry->parallellastscan = t;
 	}
 	tabentry->tuples_returned += lstats->counts.tuples_returned;
 	tabentry->tuples_fetched += lstats->counts.tuples_fetched;
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 97dc09ac0d..30a3849e3d 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -82,6 +82,9 @@ PG_STAT_GET_RELENTRY_INT64(mod_since_analyze)
 /* pg_stat_get_numscans */
 PG_STAT_GET_RELENTRY_INT64(numscans)
 
+/* pg_stat_get_parallelnumscans */
+PG_STAT_GET_RELENTRY_INT64(parallelnumscans)
+
 /* pg_stat_get_tuples_deleted */
 PG_STAT_GET_RELENTRY_INT64(tuples_deleted)
 
@@ -140,6 +143,9 @@ PG_STAT_GET_RELENTRY_TIMESTAMPTZ(last_vacuum_time)
 /* pg_stat_get_lastscan */
 PG_STAT_GET_RELENTRY_TIMESTAMPTZ(lastscan)
 
+/* pg_stat_get_parallellastscan */
+PG_STAT_GET_RELENTRY_TIMESTAMPTZ(parallellastscan)
+
 Datum
 pg_stat_get_function_calls(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index ff5436acac..1cce03a6d2 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5391,6 +5391,14 @@
   proname => 'pg_stat_get_lastscan', provolatile => 's', proparallel => 'r',
   prorettype => 'timestamptz', proargtypes => 'oid',
   prosrc => 'pg_stat_get_lastscan' },
+{ oid => '9000', descr => 'statistics: number of parallel scans done for table/index',
+  proname => 'pg_stat_get_parallelnumscans', provolatile => 's', proparallel => 'r',
+  prorettype => 'int8', proargtypes => 'oid',
+  prosrc => 'pg_stat_get_parallelnumscans' },
+{ oid => '9001', descr => 'statistics: time of the last parallel scan for table/index',
+  proname => 'pg_stat_get_parallellastscan', provolatile => 's', proparallel => 'r',
+  prorettype => 'timestamptz', proargtypes => 'oid',
+  prosrc => 'pg_stat_get_parallellastscan' },
 { oid => '1929', descr => 'statistics: number of tuples read by seqscan',
   proname => 'pg_stat_get_tuples_returned', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => 'oid',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index be2c91168a..ceca8bff7d 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -192,6 +192,7 @@ typedef struct PgStat_BackendSubEntry
 typedef struct PgStat_TableCounts
 {
 	PgStat_Counter numscans;
+	PgStat_Counter parallelnumscans;
 
 	PgStat_Counter tuples_returned;
 	PgStat_Counter tuples_fetched;
@@ -433,6 +434,8 @@ typedef struct PgStat_StatTabEntry
 {
 	PgStat_Counter numscans;
 	TimestampTz lastscan;
+	PgStat_Counter parallelnumscans;
+	TimestampTz parallellastscan;
 
 	PgStat_Counter tuples_returned;
 	PgStat_Counter tuples_fetched;
@@ -640,10 +643,13 @@ extern void pgstat_report_analyze(Relation rel,
 
 /* nontransactional event counts are simple enough to inline */
 
-#define pgstat_count_heap_scan(rel)									\
+#define pgstat_count_heap_scan(rel, parallel)						\
 	do {															\
-		if (pgstat_should_count_relation(rel))						\
+		if (pgstat_should_count_relation(rel)) {					\
 			(rel)->pgstat_info->counts.numscans++;					\
+			if (parallel)											\
+				(rel)->pgstat_info->counts.parallelnumscans++;		\
+		}															\
 	} while (0)
 #define pgstat_count_heap_getnext(rel)								\
 	do {															\
@@ -655,10 +661,13 @@ extern void pgstat_report_analyze(Relation rel,
 		if (pgstat_should_count_relation(rel))						\
 			(rel)->pgstat_info->counts.tuples_fetched++;			\
 	} while (0)
-#define pgstat_count_index_scan(rel)								\
+#define pgstat_count_index_scan(rel, parallel)						\
 	do {															\
-		if (pgstat_should_count_relation(rel))						\
+		if (pgstat_should_count_relation(rel)) {					\
 			(rel)->pgstat_info->counts.numscans++;					\
+			if (parallel)											\
+				(rel)->pgstat_info->counts.parallelnumscans++;		\
+		}															\
 	} while (0)
 #define pgstat_count_index_tuples(rel, n)							\
 	do {															\
-- 
2.46.0

Reply via email to