Le jeu. 5 sept. 2024 à 07:36, Bertrand Drouvot <bertranddrouvot...@gmail.com>
a écrit :
> Hi,
>
> On Wed, Sep 04, 2024 at 04:37:19PM +0200, Guillaume Lelarge wrote:
> > Hi,
> >
> > Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <
> bertranddrouvot...@gmail.com>
> > a écrit :
> > > What about adding a comment instead of this extra check?
> > >
> > >
> > Done too in v3.
>
> Thanks!
>
> 1 ===
>
> + /*
> + * Don't check counts.parallelnumscans because counts.numscans
> includes
> + * counts.parallelnumscans
> + */
>
> "." is missing at the end of the comment.
>
>
Fixed in v4.
> 2 ===
>
> - if (t > tabentry->lastscan)
> + if (t > tabentry->lastscan && lstats->counts.numscans)
>
> The extra check on lstats->counts.numscans is not needed as it's already
> done
> a few lines before.
>
>
Fixed in v4.
> 3 ===
>
> + if (t > tabentry->parallellastscan &&
> lstats->counts.parallelnumscans)
>
> This one makes sense.
>
> And now I'm wondering if the extra comment added in v3 is really worth it
> (and
> does not sound confusing)? I mean, the parallel check is done once we passe
> the initial test on counts.numscans. I think the code is clear enough
> without
> this extra comment, thoughts?
>
>
I'm not sure I understand you here. I kinda like the extra comment though.
> 4 ===
>
> What about adding a few tests? or do you want to wait a bit more to see if
> "
> there's an agreement on this patch" (as you stated at the start of this
> thread).
>
>
Guess I can start working on that now. It will take some time as I've never
done it before. Good thing I added the patch on the November commit fest :)
Thanks again.
Regards.
--
Guillaume.
From 6c92e70cd2698f24fe14069f675b7934e2f95bfe 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 v4] 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 | 8 +++
src/backend/utils/adt/pgstatfuncs.c | 6 ++
src/include/catalog/pg_proc.dat | 8 +++
src/include/pgstat.h | 17 +++--
13 files changed, 112 insertions(+), 18 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..766c56524e 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)
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
