v8-0002-Drop-buffers_backend_fsync-column-from-pg_stat_bg.patch

application/x-patch

Filename: v8-0002-Drop-buffers_backend_fsync-column-from-pg_stat_bg.patch
Type: application/x-patch
Part: 1
Message: Re: Introduce a new view for checkpointer related stats

Patch

Format: format-patch
Series: patch v8-0002
Subject: Drop buffers_backend_fsync column from pg_stat_bgwriter view
File+
doc/src/sgml/monitoring.sgml 0 11
src/backend/access/transam/slru.c 4 0
src/backend/catalog/system_views.sql 0 1
src/backend/postmaster/checkpointer.c 2 20
src/backend/utils/activity/pgstat_checkpointer.c 0 2
src/backend/utils/adt/pgstatfuncs.c 0 6
src/include/catalog/pg_proc.dat 0 5
src/include/pgstat.h 0 1
src/test/regress/expected/rules.out 0 1
From 09afb4d2da776b863aee9d87f475eb8cd852d44c Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 13 Feb 2023 05:02:17 +0000
Subject: [PATCH v8] Drop buffers_backend_fsync column from pg_stat_bgwriter
 view

---
 doc/src/sgml/monitoring.sgml                  | 11 ----------
 src/backend/access/transam/slru.c             |  4 ++++
 src/backend/catalog/system_views.sql          |  1 -
 src/backend/postmaster/checkpointer.c         | 22 ++-----------------
 .../utils/activity/pgstat_checkpointer.c      |  2 --
 src/backend/utils/adt/pgstatfuncs.c           |  6 -----
 src/include/catalog/pg_proc.dat               |  5 -----
 src/include/pgstat.h                          |  1 -
 src/test/regress/expected/rules.out           |  1 -
 9 files changed, 6 insertions(+), 47 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index c0cae0f4f3..098ca5999d 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4059,17 +4059,6 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
       </para></entry>
      </row>
 
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>buffers_backend_fsync</structfield> <type>bigint</type>
-      </para>
-      <para>
-       Number of times a backend had to execute its own
-       <function>fsync</function> call (normally the background writer handles those
-       even when the backend does its own write)
-      </para></entry>
-     </row>
-
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>buffers_alloc</structfield> <type>bigint</type>
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 5ab86238a9..4ae2040f39 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -1593,10 +1593,14 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data)
 int
 SlruSyncFileTag(SlruCtl ctl, const FileTag *ftag, char *path)
 {
+	SlruShared	shared = ctl->shared;
 	int			fd;
 	int			save_errno;
 	int			result;
 
+	/* update the stats counter of flushes */
+	pgstat_count_slru_flush(shared->slru_stats_idx);
+
 	SlruFileName(ctl, path, ftag->segno);
 
 	fd = OpenTransientFile(path, O_RDWR | PG_BINARY);
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 9bb96e27aa..2b8e462532 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1112,7 +1112,6 @@ CREATE VIEW pg_stat_bgwriter AS
         pg_stat_get_bgwriter_buf_written_checkpoints() AS buffers_checkpoint,
         pg_stat_get_bgwriter_buf_written_clean() AS buffers_clean,
         pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean,
-        pg_stat_get_buf_fsync_backend() AS buffers_backend_fsync,
         pg_stat_get_buf_alloc() AS buffers_alloc,
         pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
 
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index baadd08044..12c92f3de9 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -91,15 +91,11 @@
  * requesting backends since the last checkpoint start.  The flags are
  * chosen so that OR'ing is the correct way to combine multiple requests.
  *
- * num_backend_fsync counts the subset of buffer writes performed by user
- * backend processes that also had to do their own fsync, because the
- * checkpointer failed to absorb their request.
- *
  * The requests array holds fsync requests sent by backends and not yet
  * absorbed by the checkpointer.
  *
- * Unlike the checkpoint fields, num_backend_fsync and the requests fields are
- * protected by CheckpointerCommLock.
+ * Unlike the checkpoint fields, reqquests related fields are protected by
+ * CheckpointerCommLock.
  *----------
  */
 typedef struct
@@ -123,8 +119,6 @@ typedef struct
 	ConditionVariable start_cv; /* signaled when ckpt_started advances */
 	ConditionVariable done_cv;	/* signaled when ckpt_done advances */
 
-	uint32		num_backend_fsync;	/* counts user backend fsync calls */
-
 	int			num_requests;	/* current # of requests */
 	int			max_requests;	/* allocated array size */
 	CheckpointerRequest requests[FLEXIBLE_ARRAY_MEMBER];
@@ -1102,12 +1096,6 @@ ForwardSyncRequest(const FileTag *ftag, SyncRequestType type)
 		(CheckpointerShmem->num_requests >= CheckpointerShmem->max_requests &&
 		 !CompactCheckpointerRequestQueue()))
 	{
-		/*
-		 * Count the subset of writes where backends have to do their own
-		 * fsync
-		 */
-		if (!AmBackgroundWriterProcess())
-			CheckpointerShmem->num_backend_fsync++;
 		LWLockRelease(CheckpointerCommLock);
 		return false;
 	}
@@ -1264,12 +1252,6 @@ AbsorbSyncRequests(void)
 
 	LWLockAcquire(CheckpointerCommLock, LW_EXCLUSIVE);
 
-	/* Transfer stats counts into pending pgstats message */
-	PendingCheckpointerStats.buf_fsync_backend
-		+= CheckpointerShmem->num_backend_fsync;
-
-	CheckpointerShmem->num_backend_fsync = 0;
-
 	/*
 	 * We try to avoid holding the lock for a long time by copying the request
 	 * array, and processing the requests after releasing the lock.
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index a048205d6e..03ed5dddda 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -52,7 +52,6 @@ pgstat_report_checkpointer(void)
 	CHECKPOINTER_ACC(checkpoint_write_time);
 	CHECKPOINTER_ACC(checkpoint_sync_time);
 	CHECKPOINTER_ACC(buf_written_checkpoints);
-	CHECKPOINTER_ACC(buf_fsync_backend);
 #undef CHECKPOINTER_ACC
 
 	pgstat_end_changecount_write(&stats_shmem->changecount);
@@ -119,6 +118,5 @@ pgstat_checkpointer_snapshot_cb(void)
 	CHECKPOINTER_COMP(checkpoint_write_time);
 	CHECKPOINTER_COMP(checkpoint_sync_time);
 	CHECKPOINTER_COMP(buf_written_checkpoints);
-	CHECKPOINTER_COMP(buf_fsync_backend);
 #undef CHECKPOINTER_COMP
 }
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 28075fa6fb..087ba87a86 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1227,12 +1227,6 @@ pg_stat_get_bgwriter_stat_reset_time(PG_FUNCTION_ARGS)
 	PG_RETURN_TIMESTAMPTZ(pgstat_fetch_stat_bgwriter()->stat_reset_timestamp);
 }
 
-Datum
-pg_stat_get_buf_fsync_backend(PG_FUNCTION_ARGS)
-{
-	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->buf_fsync_backend);
-}
-
 Datum
 pg_stat_get_buf_alloc(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index e800fdfd5f..5ed66af0e3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5704,11 +5704,6 @@
   proname => 'pg_stat_get_checkpoint_sync_time', provolatile => 's',
   proparallel => 'r', prorettype => 'float8', proargtypes => '',
   prosrc => 'pg_stat_get_checkpoint_sync_time' },
-{ oid => '3063',
-  descr => 'statistics: number of backend buffer writes that did their own fsync',
-  proname => 'pg_stat_get_buf_fsync_backend', provolatile => 's',
-  proparallel => 'r', prorettype => 'int8', proargtypes => '',
-  prosrc => 'pg_stat_get_buf_fsync_backend' },
 { oid => '2859', descr => 'statistics: number of buffer allocations',
   proname => 'pg_stat_get_buf_alloc', provolatile => 's', proparallel => 'r',
   prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_buf_alloc' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index d1675909db..28fb6292d9 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -273,7 +273,6 @@ typedef struct PgStat_CheckpointerStats
 	PgStat_Counter checkpoint_write_time;	/* times in milliseconds */
 	PgStat_Counter checkpoint_sync_time;
 	PgStat_Counter buf_written_checkpoints;
-	PgStat_Counter buf_fsync_backend;
 } PgStat_CheckpointerStats;
 
 
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 5b4f441245..9c9adb6140 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1821,7 +1821,6 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_timed_checkpoints() AS checkpoints
     pg_stat_get_bgwriter_buf_written_checkpoints() AS buffers_checkpoint,
     pg_stat_get_bgwriter_buf_written_clean() AS buffers_clean,
     pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean,
-    pg_stat_get_buf_fsync_backend() AS buffers_backend_fsync,
     pg_stat_get_buf_alloc() AS buffers_alloc,
     pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
 pg_stat_database| SELECT oid AS datid,
-- 
2.34.1