[Shinya Kato <Shinya11.Kato@oss.nttdata.com>]

Hi, hackers

There is below description in docs for stats_fetch_consistency.
"Changing this parameter in a transaction discards the statistics 
snapshot."

However, I wonder if changes stats_fetch_consistency in a transaction, 
statistics is not discarded in some cases.

Example:
--
* session 1
=# SET stats_fetch_consistency TO snapshot;
=# BEGIN;
=*# SELECT wal_records, wal_fpi, wal_bytes FROM pg_stat_wal;
  wal_records | wal_fpi | wal_bytes
-------------+---------+-----------
        23592 |     628 |   5939027
(1 row)

* session 2
=# CREATE TABLE test (i int); -- generate WAL records
=# SELECT wal_records, wal_fpi, wal_bytes FROM pg_stat_wal;
  wal_records | wal_fpi | wal_bytes
-------------+---------+-----------
        23631 |     644 |   6023411
(1 row)

* session 1
=*# -- snapshot is not discarded, it is right
=*# SELECT wal_records, wal_fpi, wal_bytes FROM pg_stat_wal;
  wal_records | wal_fpi | wal_bytes
-------------+---------+-----------
        23592 |     628 |   5939027
(1 row)

=*# SET stats_fetch_consistency TO cache;

=*# -- snapshot is not discarded, it is not right
=*# SELECT wal_records, wal_fpi, wal_bytes FROM pg_stat_wal;
  wal_records | wal_fpi | wal_bytes
-------------+---------+-----------
        23592 |     628 |   5939027
(1 row)
--

I can see similar cases in pg_stat_archiver, pg_stat_bgwriter, 
pg_stat_checkpointer, pg_stat_io, and pg_stat_slru.
Is it a bug? I fixed it, and do you think?

-- 
Regards,
Shinya Kato
NTT DATA GROUP CORPORATION

Attachments:

v1-0001-fix-stats-fetch-consinstency-bugs.patch (text/x-diff, 3.3K) download
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 125ca97b18..40ef6ef321 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1714,3 +1714,11 @@ assign_stats_fetch_consistency(int newval, void *extra)
 	if (pgstat_fetch_consistency != newval)
 		force_stats_snapshot_clear = true;
 }
+
+
+void
+pgstat_clear_snapshot_if_needed(void)
+{
+	if (force_stats_snapshot_clear)
+		pgstat_clear_snapshot();
+}
diff --git a/src/backend/utils/activity/pgstat_archiver.c b/src/backend/utils/activity/pgstat_archiver.c
index 66398b20e5..d4979e9fd0 100644
--- a/src/backend/utils/activity/pgstat_archiver.c
+++ b/src/backend/utils/activity/pgstat_archiver.c
@@ -57,6 +57,7 @@ pgstat_report_archiver(const char *xlog, bool failed)
 PgStat_ArchiverStats *
 pgstat_fetch_stat_archiver(void)
 {
+	pgstat_clear_snapshot_if_needed();
 	pgstat_snapshot_fixed(PGSTAT_KIND_ARCHIVER);
 
 	return &pgStatLocal.snapshot.archiver;
diff --git a/src/backend/utils/activity/pgstat_bgwriter.c b/src/backend/utils/activity/pgstat_bgwriter.c
index 7d2432e4fa..931d0f17e7 100644
--- a/src/backend/utils/activity/pgstat_bgwriter.c
+++ b/src/backend/utils/activity/pgstat_bgwriter.c
@@ -70,6 +70,7 @@ pgstat_report_bgwriter(void)
 PgStat_BgWriterStats *
 pgstat_fetch_stat_bgwriter(void)
 {
+	pgstat_clear_snapshot_if_needed();
 	pgstat_snapshot_fixed(PGSTAT_KIND_BGWRITER);
 
 	return &pgStatLocal.snapshot.bgwriter;
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index 30a8110e38..ac700072ed 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -79,6 +79,7 @@ pgstat_report_checkpointer(void)
 PgStat_CheckpointerStats *
 pgstat_fetch_stat_checkpointer(void)
 {
+	pgstat_clear_snapshot_if_needed();
 	pgstat_snapshot_fixed(PGSTAT_KIND_CHECKPOINTER);
 
 	return &pgStatLocal.snapshot.checkpointer;
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index 43c393d6fe..13712aa4af 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -156,6 +156,7 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 PgStat_IO *
 pgstat_fetch_stat_io(void)
 {
+	pgstat_clear_snapshot_if_needed();
 	pgstat_snapshot_fixed(PGSTAT_KIND_IO);
 
 	return &pgStatLocal.snapshot.io;
diff --git a/src/backend/utils/activity/pgstat_slru.c b/src/backend/utils/activity/pgstat_slru.c
index 56ea1c3378..7f8a97e8ca 100644
--- a/src/backend/utils/activity/pgstat_slru.c
+++ b/src/backend/utils/activity/pgstat_slru.c
@@ -104,6 +104,7 @@ pgstat_count_slru_truncate(int slru_idx)
 PgStat_SLRUStats *
 pgstat_fetch_slru(void)
 {
+	pgstat_clear_snapshot_if_needed();
 	pgstat_snapshot_fixed(PGSTAT_KIND_SLRU);
 
 	return pgStatLocal.snapshot.slru;
diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c
index 1a3c0e1a66..f8fcf909f0 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -66,6 +66,7 @@ pgstat_report_wal(bool force)
 PgStat_WalStats *
 pgstat_fetch_stat_wal(void)
 {
+	pgstat_clear_snapshot_if_needed();
 	pgstat_snapshot_fixed(PGSTAT_KIND_WAL);
 
 	return &pgStatLocal.snapshot.wal;