From 479fdfc53d1a6ee9943bdc580884866e99497673 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 2 Sep 2021 11:47:41 -0400 Subject: [PATCH v8 2/2] 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 | 6 +--- src/backend/postmaster/checkpointer.c | 26 --------------- 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, 1 insertion(+), 156 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 60627c692a..08772652ac 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3416,24 +3416,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 @@ -3444,35 +3426,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 30280d520b..c45c261f4b 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1058,18 +1058,14 @@ CREATE VIEW pg_stat_archiver AS s.stats_reset FROM pg_stat_get_archiver() s; +-- TODO: make separate pg_stat_checkpointer view CREATE VIEW pg_stat_bgwriter AS SELECT pg_stat_get_bgwriter_timed_checkpoints() AS checkpoints_timed, 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 fe6bd15506..6d3a8da948 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]; @@ -1087,10 +1076,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 @@ -1104,8 +1089,6 @@ ForwardSyncRequest(const FileTag *ftag, SyncRequestType type) * Count the subset of writes where backends have to do their own * fsync */ - if (!AmBackgroundWriterProcess()) - CheckpointerShmem->num_backend_fsync++; pgstat_increment_buffer_access_type(BA_Fsync, Buf_Shared); LWLockRelease(CheckpointerCommLock); return false; @@ -1263,15 +1246,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 40f646b7c6..849a9be702 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -5586,9 +5586,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; } /* ---------- @@ -5604,9 +5602,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 135b2d3925..f3c0fd96b3 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2177,7 +2177,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++; } } @@ -2286,9 +2285,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 @@ -2485,8 +2481,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 d4e0ce4143..94a4500ead 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); -} - Datum pg_stat_get_buffers_accesses(PG_FUNCTION_ARGS) { diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 54661e2b5f..02f624c18c 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5600,16 +5600,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', @@ -5629,18 +5619,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 various types of accesses of buffers done by each backend type', proname => 'pg_stat_get_buffers_accesses', provolatile => 's', proisstrict => 'f', diff --git a/src/include/pgstat.h b/src/include/pgstat.h index c97b6897cf..9b4217cbbb 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -489,9 +489,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; /* ---------- @@ -504,9 +502,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; @@ -884,9 +879,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; @@ -900,9 +893,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 9172b0fcd2..ac2f7cf61e 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.buffer_type, -- 2.27.0