From 9f22da9041e1e1fbc0ef003f5f78f4e72274d438 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 24 Nov 2021 12:20:10 -0500 Subject: [PATCH v17 6/7] Remove superfluous bgwriter stats Remove stats from pg_stat_bgwriter which are now more clearly expressed in pg_stat_buffers. TODO: - make pg_stat_checkpointer view and move relevant stats into it - add additional stats to pg_stat_bgwriter --- doc/src/sgml/monitoring.sgml | 47 --------------------------- src/backend/catalog/system_views.sql | 5 --- src/backend/postmaster/checkpointer.c | 29 ++--------------- src/backend/postmaster/pgstat.c | 5 --- src/backend/storage/buffer/bufmgr.c | 6 ---- src/backend/utils/adt/pgstatfuncs.c | 30 ----------------- src/include/catalog/pg_proc.dat | 22 ------------- src/include/pgstat.h | 10 ------ src/test/regress/expected/rules.out | 5 --- 9 files changed, 2 insertions(+), 157 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index b16952d439..9e869a7fbf 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3551,24 +3551,6 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i - - - buffers_checkpoint bigint - - - Number of buffers written during checkpoints - - - - - - buffers_clean bigint - - - Number of buffers written by the background writer - - - maxwritten_clean bigint @@ -3579,35 +3561,6 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i - - - 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 - - - Number of buffers allocated - - - stats_reset timestamp with time zone diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index e214d23056..80d73c40ad 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1068,12 +1068,7 @@ CREATE VIEW pg_stat_bgwriter AS pg_stat_get_bgwriter_requested_checkpoints() AS checkpoints_req, pg_stat_get_checkpoint_write_time() AS checkpoint_write_time, pg_stat_get_checkpoint_sync_time() AS checkpoint_sync_time, - 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; CREATE VIEW pg_stat_buffers AS diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 8440b2b802..b9c3745474 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -90,17 +90,9 @@ * 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. *---------- */ typedef struct @@ -124,9 +116,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]; @@ -1081,10 +1070,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 @@ -1094,13 +1079,12 @@ ForwardSyncRequest(const FileTag *ftag, SyncRequestType type) (CheckpointerShmem->num_requests >= CheckpointerShmem->max_requests && !CompactCheckpointerRequestQueue())) { + LWLockRelease(CheckpointerCommLock); + /* * Count the subset of writes where backends have to do their own * fsync */ - if (!AmBackgroundWriterProcess()) - CheckpointerShmem->num_backend_fsync++; - LWLockRelease(CheckpointerCommLock); pgstat_inc_ioop(IOOP_FSYNC, IOPATH_SHARED); return false; } @@ -1257,15 +1241,6 @@ AbsorbSyncRequests(void) LWLockAcquire(CheckpointerCommLock, LW_EXCLUSIVE); - /* Transfer stats counts into pending pgstats message */ - PendingCheckpointerStats.m_buf_written_backend - += CheckpointerShmem->num_backend_writes; - PendingCheckpointerStats.m_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/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 9b041cb1b9..b1ea10a77a 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -5945,9 +5945,7 @@ pgstat_recv_archiver(PgStat_MsgArchiver *msg, int len) static void pgstat_recv_bgwriter(PgStat_MsgBgWriter *msg, int len) { - globalStats.bgwriter.buf_written_clean += msg->m_buf_written_clean; globalStats.bgwriter.maxwritten_clean += msg->m_maxwritten_clean; - globalStats.bgwriter.buf_alloc += msg->m_buf_alloc; } /* ---------- @@ -5963,9 +5961,6 @@ pgstat_recv_checkpointer(PgStat_MsgCheckpointer *msg, int len) globalStats.checkpointer.requested_checkpoints += msg->m_requested_checkpoints; globalStats.checkpointer.checkpoint_write_time += msg->m_checkpoint_write_time; globalStats.checkpointer.checkpoint_sync_time += msg->m_checkpoint_sync_time; - globalStats.checkpointer.buf_written_checkpoints += msg->m_buf_written_checkpoints; - globalStats.checkpointer.buf_written_backend += msg->m_buf_written_backend; - globalStats.checkpointer.buf_fsync_backend += msg->m_buf_fsync_backend; } static void diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 6926fc5742..67447f997a 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2164,7 +2164,6 @@ BufferSync(int flags) if (SyncOneBuffer(buf_id, false, &wb_context) & BUF_WRITTEN) { TRACE_POSTGRESQL_BUFFER_SYNC_WRITTEN(buf_id); - PendingCheckpointerStats.m_buf_written_checkpoints++; num_written++; } } @@ -2273,9 +2272,6 @@ BgBufferSync(WritebackContext *wb_context) */ strategy_buf_id = StrategySyncStart(&strategy_passes, &recent_alloc); - /* Report buffer alloc counts to pgstat */ - PendingBgWriterStats.m_buf_alloc += recent_alloc; - /* * If we're not running the LRU scan, just stop after doing the stats * stuff. We mark the saved state invalid so that we can recover sanely @@ -2472,8 +2468,6 @@ BgBufferSync(WritebackContext *wb_context) reusable_buffers++; } - PendingBgWriterStats.m_buf_written_clean += num_written; - #ifdef BGW_DEBUG elog(DEBUG1, "bgwriter: recent_alloc=%u smoothed=%.2f delta=%ld ahead=%d density=%.2f reusable_est=%d upcoming_est=%d scanned=%d wrote=%d reusable=%d", recent_alloc, smoothed_alloc, strategy_delta, bufs_ahead, diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 9fd7a8cdb9..29d3dc6a79 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -1738,18 +1738,6 @@ pg_stat_get_bgwriter_requested_checkpoints(PG_FUNCTION_ARGS) PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->requested_checkpoints); } -Datum -pg_stat_get_bgwriter_buf_written_checkpoints(PG_FUNCTION_ARGS) -{ - PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->buf_written_checkpoints); -} - -Datum -pg_stat_get_bgwriter_buf_written_clean(PG_FUNCTION_ARGS) -{ - PG_RETURN_INT64(pgstat_fetch_stat_bgwriter()->buf_written_clean); -} - Datum pg_stat_get_bgwriter_maxwritten_clean(PG_FUNCTION_ARGS) { @@ -1778,24 +1766,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) -{ - PG_RETURN_INT64(pgstat_fetch_stat_bgwriter()->buf_alloc); -} - /* * When adding a new column to the pg_stat_buffers view, add a new enum * value here above COLUMN_LENGTH. diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 32253383ba..e303efa798 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5612,16 +5612,6 @@ proname => 'pg_stat_get_bgwriter_requested_checkpoints', provolatile => 's', proparallel => 'r', prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_bgwriter_requested_checkpoints' }, -{ oid => '2771', - descr => 'statistics: number of buffers written by the bgwriter during checkpoints', - proname => 'pg_stat_get_bgwriter_buf_written_checkpoints', provolatile => 's', - proparallel => 'r', prorettype => 'int8', proargtypes => '', - prosrc => 'pg_stat_get_bgwriter_buf_written_checkpoints' }, -{ oid => '2772', - descr => 'statistics: number of buffers written by the bgwriter for cleaning dirty buffers', - proname => 'pg_stat_get_bgwriter_buf_written_clean', provolatile => 's', - proparallel => 'r', prorettype => 'int8', proargtypes => '', - prosrc => 'pg_stat_get_bgwriter_buf_written_clean' }, { oid => '2773', descr => 'statistics: number of times the bgwriter stopped processing when it had written too many buffers while cleaning', proname => 'pg_stat_get_bgwriter_maxwritten_clean', provolatile => 's', @@ -5641,18 +5631,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' }, { oid => '8459', descr => 'statistics: counts of all IO operations done to all IO paths by each type of backend.', proname => 'pg_stat_get_buffers', provolatile => 's', proisstrict => 'f', diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 9e1c635108..865dd6d201 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -513,9 +513,7 @@ typedef struct PgStat_MsgBgWriter { PgStat_MsgHdr m_hdr; - PgStat_Counter m_buf_written_clean; PgStat_Counter m_maxwritten_clean; - PgStat_Counter m_buf_alloc; } PgStat_MsgBgWriter; /* ---------- @@ -528,9 +526,6 @@ typedef struct PgStat_MsgCheckpointer PgStat_Counter m_timed_checkpoints; PgStat_Counter m_requested_checkpoints; - PgStat_Counter m_buf_written_checkpoints; - PgStat_Counter m_buf_written_backend; - PgStat_Counter m_buf_fsync_backend; PgStat_Counter m_checkpoint_write_time; /* times in milliseconds */ PgStat_Counter m_checkpoint_sync_time; } PgStat_MsgCheckpointer; @@ -961,9 +956,7 @@ typedef struct PgStat_ArchiverStats */ typedef struct PgStat_BgWriterStats { - PgStat_Counter buf_written_clean; PgStat_Counter maxwritten_clean; - PgStat_Counter buf_alloc; TimestampTz stat_reset_timestamp; } PgStat_BgWriterStats; @@ -977,9 +970,6 @@ typedef struct PgStat_CheckpointerStats PgStat_Counter requested_checkpoints; 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 5869ce442f..09f495792d 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1821,12 +1821,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_timed_checkpoints() AS checkpoints pg_stat_get_bgwriter_requested_checkpoints() AS checkpoints_req, pg_stat_get_checkpoint_write_time() AS checkpoint_write_time, pg_stat_get_checkpoint_sync_time() AS checkpoint_sync_time, - 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_buffers| SELECT b.backend_type, b.io_path, -- 2.32.0