From 74604a37f2f9eb6e626a4ff3cedd02aef5a2ad59 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 27 Oct 2023 11:16:39 +0900 Subject: [PATCH] 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.mcszbscsqs3bc5nx@awork3.anarazel.de --- doc/src/sgml/monitoring.sgml | 20 ------------ src/backend/catalog/system_views.sql | 2 -- src/backend/postmaster/checkpointer.c | 32 ++----------------- .../utils/activity/pgstat_checkpointer.c | 4 --- src/backend/utils/adt/pgstatfuncs.c | 12 ------- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 9 ------ src/include/pgstat.h | 2 -- src/test/regress/expected/rules.out | 2 -- 9 files changed, 3 insertions(+), 82 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 3f49ff79f3434..9fea60b5b211b 100644 --- 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 - - - buffers_backend bigint - - - Number of buffers written directly by a backend - - - - - - buffers_backend_fsync bigint - - - Number of times a backend had to execute its own - fsync call (normally the background writer handles those - even when the backend does its own write) - - - buffers_alloc bigint diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index fcb14976c057b..886f175fc2d94 100644 --- 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 ace9893d95767..4fe403c9a89fa 100644 --- 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 26dec112f6cd6..03ed5dddda7b8 100644 --- 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 3b44af8006649..6468b6a80574d 100644 --- 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 f9e1c0e5351fd..d75acb3b089c7 100644 --- 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 06435e8b92565..bc41e926776a0 100644 --- 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 fe2642f71a44e..e6fd20f1ce2bc 100644 --- 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 2c60400adefe0..798d1610f25de 100644 --- 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,