Remove buffers_backend and buffers_backend_fsync from pg_stat_checkpointer

Two attributes related to checkpointer statistics are removed in this
commit:
- buffers_backend, that counts the number of buffers written directly by
a backend.
- buffers_backend_fsync, that counts the number of times a backend had
to do fsync() by its own.

These are actually not checkpointer properties but backend properties.
Also, pg_stat_io provides a more accurate and equivalent report of these
numbers, by tracking all the I/O stats related to backends, including
writes and fsyncs, so storing them in pg_stat_checkpointer was
redundant.

Thanks also to Robert Haas and Amit Kapila for their input.

Bump catalog version.

Author: Bharath Rupireddy
Reviewed-by: Bertrand Drouvot, Andres Freund
Discussion: https://postgr.es/m/20230210004604[email protected]

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 3f49ff79f3434c7fa1b299cdd7a6e917361ffeac..9fea60b5b211b5782cb7e576a597a7a47504a0e7 100644 (file)
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2953,26 +2953,6 @@ description | Waiting for a newly initialized WAL file to reach durable storage
       </para></entry>
      </row>
 
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>buffers_backend</structfield> <type>bigint</type>
-      </para>
-      <para>
-       Number of buffers written directly by a backend
-      </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/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index fcb14976c057b49151cb3389d02358f5898e5205..886f175fc2d943f2fffe3e94067b40e86744cc4b 100644 (file)
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1118,8 +1118,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_written_backend() AS buffers_backend,
-        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 ace9893d957674ef5a1a251d949b66555057f8a8..4fe403c9a89fa706ac88bb5bbe3b09332c73f8d4 100644 (file)
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -91,17 +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_writes is used to count the number of buffer writes performed
- * by user backend processes.  This counter should be wide enough that it
- * can't overflow during a single processing cycle.  num_backend_fsync
- * counts the subset of those writes 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_writes, num_backend_fsync, and
- * the requests fields are protected by CheckpointerCommLock.
+ * Unlike the checkpoint fields, requests related fields are protected by
+ * CheckpointerCommLock.
  *----------
  */
 typedef struct
@@ -125,9 +119,6 @@ typedef struct
        ConditionVariable start_cv; /* signaled when ckpt_started advances */
        ConditionVariable done_cv;      /* signaled when ckpt_done advances */
 
-       uint32          num_backend_writes; /* counts user backend buffer writes */
-       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];
@@ -1095,10 +1086,6 @@ ForwardSyncRequest(const FileTag *ftag, SyncRequestType type)
 
        LWLockAcquire(CheckpointerCommLock, LW_EXCLUSIVE);
 
-       /* Count all backend writes regardless of if they fit in the queue */
-       if (!AmBackgroundWriterProcess())
-               CheckpointerShmem->num_backend_writes++;
-
        /*
         * If the checkpointer isn't running or the request queue is full, the
         * backend will have to perform its own fsync request.  But before forcing
@@ -1108,12 +1095,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;
        }
@@ -1270,15 +1251,6 @@ AbsorbSyncRequests(void)
 
        LWLockAcquire(CheckpointerCommLock, LW_EXCLUSIVE);
 
-       /* Transfer stats counts into pending pgstats message */
-       PendingCheckpointerStats.buf_written_backend
-               += CheckpointerShmem->num_backend_writes;
-       PendingCheckpointerStats.buf_fsync_backend
-               += CheckpointerShmem->num_backend_fsync;
-
-       CheckpointerShmem->num_backend_writes = 0;
-       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 26dec112f6cd6d79789c569cb1b3c23a37cc4eb0..03ed5dddda7b86d6875f98eab755dd6e6b56cbca 100644 (file)
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -52,8 +52,6 @@ pgstat_report_checkpointer(void)
        CHECKPOINTER_ACC(checkpoint_write_time);
        CHECKPOINTER_ACC(checkpoint_sync_time);
        CHECKPOINTER_ACC(buf_written_checkpoints);
-       CHECKPOINTER_ACC(buf_written_backend);
-       CHECKPOINTER_ACC(buf_fsync_backend);
 #undef CHECKPOINTER_ACC
 
        pgstat_end_changecount_write(&stats_shmem->changecount);
@@ -120,7 +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_written_backend);
-       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 3b44af80066498699e548b31835521b13f4fa722..6468b6a80574d03f9ba27a5479a7355103e26adf 100644 (file)
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1233,18 +1233,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_written_backend(PG_FUNCTION_ARGS)
-{
-       PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->buf_written_backend);
-}
-
-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/catversion.h b/src/include/catalog/catversion.h
index f9e1c0e5351fd43181002e67139768fe1797460b..d75acb3b089c79846b63a04539546411c929873c 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     202310261
+#define CATALOG_VERSION_NO     202310271
 
 #endif

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 06435e8b925655bf9e786b1481915cb8f3f2aed5..bc41e926776a025c3c67ade0dfb0e8761c274d1c 100644 (file)
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5738,15 +5738,6 @@
   proname => 'pg_stat_get_checkpoint_sync_time', provolatile => 's',
   proparallel => 'r', prorettype => 'float8', proargtypes => '',
   prosrc => 'pg_stat_get_checkpoint_sync_time' },
-{ oid => '2775', descr => 'statistics: number of buffers written by backends',
-  proname => 'pg_stat_get_buf_written_backend', provolatile => 's',
-  proparallel => 'r', prorettype => 'int8', proargtypes => '',
-  prosrc => 'pg_stat_get_buf_written_backend' },
-{ 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 fe2642f71a44e6013b7b0054614d8a83dca8748b..e6fd20f1ce2bc684c9199babc197bbb6ccdd13ee 100644 (file)
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -265,8 +265,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_written_backend;
-       PgStat_Counter buf_fsync_backend;
 } PgStat_CheckpointerStats;
 
 

diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 2c60400adefe02a2a82d73b20c4be4196d09f1d5..798d1610f25de5480588d0605d090798f8a3e6a1 100644 (file)
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1823,8 +1823,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_written_backend() AS buffers_backend,
-    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,