On 2020/11/16 12:22, Seino Yuki wrote:
Thanks for updating the patch!
+ pgss_info->dealloc = 0;
+ SpinLockInit(&pgss_info->mutex);
+ Assert(pgss_info->dealloc == 0);
Why is this assertion check necessary? It seems not necessary.
+ {
+ Assert(found == found_info);
Having pgssSharedState and pgssInfoCounters separately might make
the code a bit more complicated like the above? If this is true, what about
including pgssInfoCounters in pgssSharedState?
PGSS_FILE_HEADER needs to be changed since the patch changes
the format of pgss file?
+ /* Read pgss_info */
+ if (feof(file) == 0)
+ if (fread(pgss_info, sizeof(pgssInfoCounters), 1, file) != 1)
+ goto read_error;
Why does feof(file) need to be called here?
+pgss_info_update(void)
+{
+ {
Why is the second "{" necessary? It seems redundant.
+pgss_info_reset(void)
+{
+ {
Same as above.
+pg_stat_statements_info(PG_FUNCTION_ARGS)
+{
+ int64 d_count = 0;
+ {
Same as above.
+ SpinLockAcquire(&c->mutex);
+ d_count = Int64GetDatum(c->dealloc);
+ SpinLockRelease(&c->mutex);
Why does Int64GetDatum() need to be called here? It seems not necessary.
+ <varlistentry>
+ <term>
+ <function>pg_stat_statements_info() returns bigint</function>
+ <indexterm>
+ <primary>pg_stat_statements_info</primary>
+ </indexterm>
+ </term>
Isn't it better not to expose pg_stat_statements_info() function in the
document because pg_stat_statements_info view is enough and there
seems no use case for the function?
Regards,
Thanks for the comment.
I'll post a fixed patch.
Due to similar fixed, we have also merged the patches discussed in the
following thread.
https://commitfest.postgresql.org/30/2738/
I agree that these two patches should use the same infrastructure
because they both try to add the global stats for pg_stat_statements.
But IMO they should not be merged to one patch. It's better to
develop them one by one for ease of review. Thought?
So I extracted the "dealloc" part from the merged version of your patch.
Also I refactored the code and applied some cosmetic changes into
the patch. Attached is the updated version of the patch that implements
only "dealloc" part. Could you review this version?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/contrib/pg_stat_statements/Makefile
b/contrib/pg_stat_statements/Makefile
index 081f997d70..3ec627b956 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -6,7 +6,7 @@ OBJS = \
pg_stat_statements.o
EXTENSION = pg_stat_statements
-DATA = pg_stat_statements--1.4.sql \
+DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql \
pg_stat_statements--1.7--1.8.sql pg_stat_statements--1.6--1.7.sql \
pg_stat_statements--1.5--1.6.sql pg_stat_statements--1.4--1.5.sql \
pg_stat_statements--1.3--1.4.sql pg_stat_statements--1.2--1.3.sql \
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out
b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index 2a303a7f07..16158525ca 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -861,4 +861,19 @@ SELECT query, plans, calls, rows FROM pg_stat_statements
ORDER BY query COLLATE
SELECT query, plans, calls, rows FROM pg_stat_statements ORDER BY query
COLLATE "C" | 1 | 0 | 0
(6 rows)
+--
+-- access to pg_stat_statements_info view
+--
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT dealloc FROM pg_stat_statements_info;
+ dealloc
+---------
+ 0
+(1 row)
+
DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
new file mode 100644
index 0000000000..2019a4ffe0
--- /dev/null
+++ b/contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql
@@ -0,0 +1,17 @@
+/* contrib/pg_stat_statements/pg_stat_statements--1.8--1.9.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pg_stat_statements UPDATE TO '1.9'" to load this
file. \quit
+
+--- Define pg_stat_statements_info
+CREATE FUNCTION pg_stat_statements_info(
+ OUT dealloc bigint
+)
+RETURNS bigint
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT VOLATILE PARALLEL SAFE;
+
+CREATE VIEW pg_stat_statements_info AS
+ SELECT * FROM pg_stat_statements_info();
+
+GRANT SELECT ON pg_stat_statements_info TO PUBLIC;
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c
b/contrib/pg_stat_statements/pg_stat_statements.c
index dd963c4644..9de2d53496 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -98,7 +98,7 @@ PG_MODULE_MAGIC;
#define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat"
/* Magic number identifying the stats file format */
-static const uint32 PGSS_FILE_HEADER = 0x20171004;
+static const uint32 PGSS_FILE_HEADER = 0x20201116;
/* PostgreSQL major version number, changes in which invalidate all entries */
static const uint32 PGSS_PG_MAJOR_VERSION = PG_VERSION_NUM / 100;
@@ -193,6 +193,14 @@ typedef struct Counters
uint64 wal_bytes; /* total amount of WAL bytes
generated */
} Counters;
+/*
+ * Global statistics for pg_stat_statements
+ */
+typedef struct pgssGlobalStats
+{
+ int64 dealloc; /* # of times entries were
deallocated */
+} pgssGlobalStats;
+
/*
* Statistics per statement
*
@@ -222,6 +230,7 @@ typedef struct pgssSharedState
Size extent; /* current extent of query file
*/
int n_writers; /* number of active
writers to query file */
int gc_count; /* query file garbage
collection cycle count */
+ pgssGlobalStats stats; /* global statistics for pgss */
} pgssSharedState;
/*
@@ -327,6 +336,7 @@ PG_FUNCTION_INFO_V1(pg_stat_statements_1_2);
PG_FUNCTION_INFO_V1(pg_stat_statements_1_3);
PG_FUNCTION_INFO_V1(pg_stat_statements_1_8);
PG_FUNCTION_INFO_V1(pg_stat_statements);
+PG_FUNCTION_INFO_V1(pg_stat_statements_info);
static void pgss_shmem_startup(void);
static void pgss_shmem_shutdown(int code, Datum arg);
@@ -554,6 +564,7 @@ pgss_shmem_startup(void)
pgss->extent = 0;
pgss->n_writers = 0;
pgss->gc_count = 0;
+ pgss->stats.dealloc = 0;
}
memset(&info, 0, sizeof(info));
@@ -673,6 +684,10 @@ pgss_shmem_startup(void)
entry->counters = temp.counters;
}
+ /* Read global statistics for pg_stat_statements */
+ if (fread(&pgss->stats, sizeof(pgssGlobalStats), 1, file) != 1)
+ goto read_error;
+
pfree(buffer);
FreeFile(file);
FreeFile(qfile);
@@ -794,6 +809,10 @@ pgss_shmem_shutdown(int code, Datum arg)
}
}
+ /* Dump global statistics for pg_stat_statements */
+ if (fwrite(&pgss->stats, sizeof(pgssGlobalStats), 1, file) != 1)
+ goto error;
+
free(qbuffer);
qbuffer = NULL;
@@ -1863,6 +1882,26 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
tuplestore_donestoring(tupstore);
}
+/*
+ * Return statistics of pg_stat_statements.
+ */
+Datum
+pg_stat_statements_info(PG_FUNCTION_ARGS)
+{
+ pgssGlobalStats stats;
+
+ /* Read global statistics for pg_stat_statements */
+ {
+ volatile pgssSharedState *s = (volatile pgssSharedState *) pgss;
+
+ SpinLockAcquire(&s->mutex);
+ stats = s->stats;
+ SpinLockRelease(&s->mutex);
+ }
+
+ PG_RETURN_INT64(stats.dealloc);
+}
+
/*
* Estimate shared memory space needed.
*/
@@ -2018,6 +2057,15 @@ entry_dealloc(void)
}
pfree(entries);
+
+ /* Increment the number of times entries are deallocated */
+ {
+ volatile pgssSharedState *s = (volatile pgssSharedState *) pgss;
+
+ SpinLockAcquire(&s->mutex);
+ s->stats.dealloc += 1;
+ SpinLockRelease(&s->mutex);
+ }
}
/*
@@ -2504,6 +2552,15 @@ entry_reset(Oid userid, Oid dbid, uint64 queryid)
hash_search(pgss_hash, &entry->key, HASH_REMOVE, NULL);
num_remove++;
}
+
+ /* Reset global statistics for pg_stat_statements */
+ {
+ volatile pgssSharedState *s = (volatile pgssSharedState
*) pgss;
+
+ SpinLockAcquire(&s->mutex);
+ s->stats.dealloc = 0;
+ SpinLockRelease(&s->mutex);
+ }
}
/* All entries are removed? */
diff --git a/contrib/pg_stat_statements/pg_stat_statements.control
b/contrib/pg_stat_statements/pg_stat_statements.control
index 65b18b11d2..2f1ce6ed50 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.control
+++ b/contrib/pg_stat_statements/pg_stat_statements.control
@@ -1,5 +1,5 @@
# pg_stat_statements extension
comment = 'track planning and execution statistics of all SQL statements
executed'
-default_version = '1.8'
+default_version = '1.9'
module_pathname = '$libdir/pg_stat_statements'
relocatable = true
diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
index e9f5bb84e3..6f58d9d0f6 100644
--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
@@ -358,4 +358,10 @@ SELECT 42;
SELECT 42;
SELECT query, plans, calls, rows FROM pg_stat_statements ORDER BY query
COLLATE "C";
+--
+-- access to pg_stat_statements_info view
+--
+SELECT pg_stat_statements_reset();
+SELECT dealloc FROM pg_stat_statements_info;
+
DROP EXTENSION pg_stat_statements;
diff --git a/doc/src/sgml/pgstatstatements.sgml
b/doc/src/sgml/pgstatstatements.sgml
index cf2d25b7b2..b0c8d6f0f1 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -23,7 +23,9 @@
<para>
When <filename>pg_stat_statements</filename> is loaded, it tracks
statistics across all databases of the server. To access and manipulate
- these statistics, the module provides a view,
<structname>pg_stat_statements</structname>,
+ these statistics, the module provides views
+ <structname>pg_stat_statements</structname> and
+ <structname>pg_stat_statements_info</structname>,
and the utility functions <function>pg_stat_statements_reset</function> and
<function>pg_stat_statements</function>. These are not available globally
but
can be enabled for a specific database with
@@ -480,6 +482,50 @@
</para>
</sect2>
+ <sect2>
+ <title>The <structname>pg_stat_statements_info</structname> View</title>
+
+ <indexterm>
+ <primary>pg_stat_statements_info</primary>
+ </indexterm>
+
+ <para>
+ The statistics of the <filename>pg_stat_statements</filename> module
+ itself are tracked and made available via a view named
+ <structname>pg_stat_statements_info</structname>. This view contains
+ only a single row. The columns of the view are shown in
+ <xref linkend="pgstatstatementsinfo-columns"/>.
+ </para>
+
+ <table id="pgstatstatementsinfo-columns">
+ <title><structname>pg_stat_statements_info</structname> Columns</title>
+ <tgroup cols="1">
+ <thead>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ Column Type
+ </para>
+ <para>
+ Description
+ </para></entry>
+ </row>
+ </thead>
+
+ <tbody>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>dealloc</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total number of times <structname>pg_stat_statements</structname>
+ entries were deallocated
+ </para></entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+ </sect2>
+
<sect2>
<title>Functions</title>
@@ -501,9 +547,10 @@
specified, the default value <literal>0</literal>(invalid) is used for
each of them and the statistics that match with other parameters will be
reset. If no parameter is specified or all the specified parameters are
- <literal>0</literal>(invalid), it will discard all statistics. By
- default, this function can only be executed by superusers. Access may be
- granted to others using <command>GRANT</command>.
+ <literal>0</literal>(invalid), it will discard all statistics including
+ the statistics that <structname>pg_stat_statements_info</structname>
+ displays. By default, this function can only be executed by superusers.
+ Access may be granted to others using <command>GRANT</command>.
</para>
</listitem>
</varlistentry>
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index b146b3ea73..65f59ea09f 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3215,6 +3215,7 @@ pgpid_t
pgsocket
pgsql_thing_t
pgssEntry
+pgssGlobalStats
pgssHashKey
pgssJumbleState
pgssLocationLen
