I apologize for not being active on this thread. However, I have now
returned to the thread and confirmed that the inconsistency is still
present in the latest code. I believe it’s crucial to address this
issue, and I am currently submitting the v5 version of the patch. The
v4 version had addressed the feedback from Bharath, Kyotaro, Andres,
and Robert. The current version has been rebased to incorporate
Vignesh’s suggestions. In response to Michael’s comments, I’ve moved
the new ‘slru_written’ column from the ‘pg_stat_bgwriter’ view to the
‘pg_stat_checkpointer’ in the attached patch.
To summarize our discussions, we’ve reached a consensus to correct the
mismatch between the information on buffers written as displayed in
the ‘pg_stat_checkpointer’ view and the checkpointer log message.
We’ve also agreed to separate the SLRU buffers data from the buffers
written and present the SLRU buffers data in a distinct field.
I have created the new commitfest entry here
https://commitfest.postgresql.org/49/5130/.
Kindly share if any comments.
Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft
On Tue, Jan 30, 2024 at 1:20 PM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Sat, Jan 20, 2024 at 08:10:03AM +0530, vignesh C wrote:
> > The patch does not apply anymore, please post a rebased version of the
> > patch :
>
> There is more to it. Some of the columns of pg_stat_bgwriter have
> been moved to a different view, aka pg_stat_checkpointer. I have
> marked the patch as returned with feedback for now, so feel free to
> resubmit if you can get a new version of the patch.
> --
> Michael
From f17300193ab350c20100c8101bc2ca5f74b5c109 Mon Sep 17 00:00:00 2001
From: Nitin Jadhav <nitinjadhav@microsoft.com>
Date: Tue, 16 Jul 2024 08:32:00 +0000
Subject: [PATCH] Fix inconsistency in reporting checkpointer stats.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The information about buffers written, which is revealed through the
pg_stat_checkpointer view, has been found to be inconsistent with the
data in the checkpoint complete log message. To address this, a new
column named ‘slru_written’ has been added to the pg_stat_checkpointer
view. This column displays the SLRU buffers written during checkpoints.
The existing ‘buffers_written’ column continues to represent the main
buffers written during checkpoints. Furthermore, the information about
buffers written, which is logged in the checkpoint complete log message,
includes both the main buffer count and the SLRU buffer count. This
patch distinguishes between these two types of information.
---
doc/src/sgml/monitoring.sgml | 9 ++++++
src/backend/access/transam/slru.c | 5 +++-
src/backend/access/transam/xlog.c | 28 +++++++++++--------
src/backend/catalog/system_views.sql | 1 +
.../utils/activity/pgstat_checkpointer.c | 1 +
src/backend/utils/adt/pgstatfuncs.c | 6 ++++
src/include/access/xlog.h | 1 +
src/include/catalog/pg_proc.dat | 5 ++++
src/include/pgstat.h | 1 +
src/test/regress/expected/rules.out | 1 +
10 files changed, 45 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 55417a6fa9..b2fe425268 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3049,6 +3049,15 @@ 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>slru_written</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of SLRU buffers written during checkpoints and restartpoints
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>stats_reset</structfield> <type>timestamp with time zone</type>
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 77b05cc0a7..de59bcdf1c 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -703,7 +703,10 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata)
/* If part of a checkpoint, count this as a buffer written. */
if (fdata)
- CheckpointStats.ckpt_bufs_written++;
+ {
+ CheckpointStats.ckpt_slru_written++;
+ PendingCheckpointerStats.slru_written++;
+ }
}
/*
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 33e27a6e72..19d7fbf70a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6666,14 +6666,16 @@ LogCheckpointEnd(bool restartpoint)
*/
if (restartpoint)
ereport(LOG,
- (errmsg("restartpoint complete: wrote %d buffers (%.1f%%); "
- "%d WAL file(s) added, %d removed, %d recycled; "
- "write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
- "sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
- "distance=%d kB, estimate=%d kB; "
- "lsn=%X/%X, redo lsn=%X/%X",
+ (errmsg("restartpoint complete: wrote %d buffers (%.1f%%), "
+ "wrote %d slru buffers (%.1f%%); %d WAL file(s) added, "
+ "%d removed, %d recycled; write=%ld.%03d s, "
+ "sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, "
+ "longest=%ld.%03d s, average=%ld.%03d s; distance=%d kB, "
+ "estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X",
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
+ CheckpointStats.ckpt_slru_written,
+ (double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
CheckpointStats.ckpt_segs_removed,
CheckpointStats.ckpt_segs_recycled,
@@ -6689,14 +6691,16 @@ LogCheckpointEnd(bool restartpoint)
LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
else
ereport(LOG,
- (errmsg("checkpoint complete: wrote %d buffers (%.1f%%); "
- "%d WAL file(s) added, %d removed, %d recycled; "
- "write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
- "sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
- "distance=%d kB, estimate=%d kB; "
- "lsn=%X/%X, redo lsn=%X/%X",
+ (errmsg("checkpoint complete: wrote %d buffers (%.1f%%), "
+ "wrote %d slru buffers (%.1f%%); %d WAL file(s) added, "
+ "%d removed, %d recycled; write=%ld.%03d s, "
+ "sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, "
+ "longest=%ld.%03d s, average=%ld.%03d s; distance=%d kB, "
+ "estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X",
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
+ CheckpointStats.ckpt_slru_written,
+ (double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
CheckpointStats.ckpt_segs_removed,
CheckpointStats.ckpt_segs_recycled,
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 19cabc9a47..aaaa2b3474 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1144,6 +1144,7 @@ CREATE VIEW pg_stat_checkpointer AS
pg_stat_get_checkpointer_write_time() AS write_time,
pg_stat_get_checkpointer_sync_time() AS sync_time,
pg_stat_get_checkpointer_buffers_written() AS buffers_written,
+ pg_stat_get_checkpointer_slru_written() AS slru_written,
pg_stat_get_checkpointer_stat_reset_time() AS stats_reset;
CREATE VIEW pg_stat_io AS
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e18..98d16c7868 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -55,6 +55,7 @@ pgstat_report_checkpointer(void)
CHECKPOINTER_ACC(write_time);
CHECKPOINTER_ACC(sync_time);
CHECKPOINTER_ACC(buffers_written);
+ CHECKPOINTER_ACC(slru_written);
#undef CHECKPOINTER_ACC
pgstat_end_changecount_write(&stats_shmem->changecount);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 3876339ee1..f3ca746818 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1215,6 +1215,12 @@ pg_stat_get_checkpointer_buffers_written(PG_FUNCTION_ARGS)
PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->buffers_written);
}
+Datum
+pg_stat_get_checkpointer_slru_written(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->slru_written);
+}
+
Datum
pg_stat_get_bgwriter_buf_written_clean(PG_FUNCTION_ARGS)
{
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 1a1f11a943..23a264ef23 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -165,6 +165,7 @@ typedef struct CheckpointStatsData
TimestampTz ckpt_end_t; /* end of checkpoint */
int ckpt_bufs_written; /* # of buffers written */
+ int ckpt_slru_written; /* # of SLRU buffers written */
int ckpt_segs_added; /* # of new xlog segments created */
int ckpt_segs_removed; /* # of xlog segments deleted */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 73d9cf8582..6adb683553 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5757,6 +5757,11 @@
proname => 'pg_stat_get_checkpointer_buffers_written', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_buffers_written' },
+{ oid => '3814',
+ descr => 'statistics: number of SLRU buffers written during checkpoints and restartpoints',
+ proname => 'pg_stat_get_checkpointer_slru_written', provolatile => 's',
+ proparallel => 'r', prorettype => 'int8', proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_slru_written' },
{ oid => '6314', descr => 'statistics: last reset for the checkpointer',
proname => 'pg_stat_get_checkpointer_stat_reset_time', provolatile => 's',
proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 6b99bb8aad..eedcc7d4a2 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -268,6 +268,7 @@ typedef struct PgStat_CheckpointerStats
PgStat_Counter write_time; /* times in milliseconds */
PgStat_Counter sync_time;
PgStat_Counter buffers_written;
+ PgStat_Counter slru_written;
TimestampTz stat_reset_timestamp;
} PgStat_CheckpointerStats;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 4c789279e5..4b1ff2d26e 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1830,6 +1830,7 @@ pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_write_time() AS write_time,
pg_stat_get_checkpointer_sync_time() AS sync_time,
pg_stat_get_checkpointer_buffers_written() AS buffers_written,
+ pg_stat_get_checkpointer_slru_written() AS slru_written,
pg_stat_get_checkpointer_stat_reset_time() AS stats_reset;
pg_stat_database| SELECT oid AS datid,
datname,
--
2.43.0
