From ad8f98a7f6b711e03471e49cb8b4ff919d27cdae Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 3 Jan 2022 15:01:10 -0500 Subject: [PATCH v21 7/8] Remove superfluous bgwriter stats code After adding io_path_stats to PgBackendStatus, all backends keep track of all IO done on all types of IO paths. When backends exit, they send their IO operations stats to the stats collector to be persisted. These statistics are available in the pg_stat_buffers view, making the buffers_checkpoint, buffers_clean, buffers_backend, buffers_backend_fsync, and buffers_alloc columns in the pg_stat_bgwriter view redundant. In order to maintain backward compatability, these columns in pg_stat_bgwriter remain and are derived from the pg_stat_buffers view. The structs used to track the statistics for these columns in the pg_stat_bgwriter view and the functions querying them have been removed. Additionally, since the "buffers" stats reset target resets both the IO operations stats structs and the bgwriter stats structs, this member of the bgwriter stats structs is no longer needed. Instead derive the stats_reset column in the pg_stat_bgwriter view from pg_stat_buffers as well. --- src/backend/catalog/system_views.sql | 28 ++++++++++----------- src/backend/postmaster/checkpointer.c | 29 ++------------------- src/backend/postmaster/pgstat.c | 7 ------ src/backend/storage/buffer/bufmgr.c | 6 ----- src/backend/utils/adt/pgstatfuncs.c | 36 --------------------------- src/include/catalog/pg_proc.dat | 26 ------------------- src/include/pgstat.h | 11 -------- src/test/regress/expected/rules.out | 24 +++++++++++++----- 8 files changed, 34 insertions(+), 133 deletions(-) diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 446a817905..d0a0c54b74 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1064,20 +1064,6 @@ CREATE VIEW pg_stat_archiver AS s.stats_reset FROM pg_stat_get_archiver() s; -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 SELECT b.backend_type, @@ -1089,6 +1075,20 @@ SELECT b.stats_reset FROM pg_stat_get_buffers() b; +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, + (SELECT write FROM pg_stat_buffers WHERE backend_type = 'checkpointer' AND io_path = 'shared') AS buffers_checkpoint, + (SELECT write FROM pg_stat_buffers WHERE backend_type = 'background writer' AND io_path = 'shared') AS buffers_clean, + pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean, + (SELECT write FROM pg_stat_buffers WHERE backend_type = 'client backend' AND io_path = 'shared') AS buffers_backend, + (SELECT fsync FROM pg_stat_buffers WHERE backend_type = 'client backend' AND io_path = 'shared') AS buffers_backend_fsync, + (SELECT alloc FROM pg_stat_buffers WHERE backend_type = 'client backend' AND io_path = 'shared') AS buffers_alloc, + (SELECT stats_reset FROM pg_stat_buffers LIMIT 1) AS stats_reset; + CREATE VIEW pg_stat_wal AS SELECT w.wal_records, diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 4e88327425..ffe8292d8f 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -91,17 +91,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 @@ -125,9 +117,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]; @@ -1086,10 +1075,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 @@ -1099,13 +1084,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; } @@ -1262,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 b1a5b15410..93656ced72 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -4473,7 +4473,6 @@ pgstat_read_statsfiles(Oid onlydb, bool permanent, bool deep) * existing statsfile). */ ts = GetCurrentTimestamp(); - globalStats.bgwriter.stat_reset_timestamp = ts; globalStats.buffers.stat_reset_timestamp = ts; archiverStats.stat_reset_timestamp = ts; walStats.stat_reset_timestamp = ts; @@ -5672,7 +5671,6 @@ pgstat_recv_resetsharedcounter(PgStat_MsgResetsharedcounter *msg, int len) memset(&globalStats.bgwriter, 0, sizeof(globalStats.bgwriter)); memset(&globalStats.checkpointer, 0, sizeof(globalStats.checkpointer)); memset(&globalStats.buffers.ops, 0, sizeof(globalStats.buffers.ops)); - globalStats.bgwriter.stat_reset_timestamp = GetCurrentTimestamp(); globalStats.buffers.stat_reset_timestamp = GetCurrentTimestamp(); memcpy(&globalStats.buffers.resets[backend_type_get_idx(backend_type)], &msg->m_backend_resets.iop.io_path_ops, @@ -5944,9 +5942,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; } /* ---------- @@ -5962,9 +5958,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 a6e446f29a..c4ee2894a0 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2166,7 +2166,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++; } } @@ -2275,9 +2274,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 @@ -2474,8 +2470,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 b0e66f89cf..15ca2d0a73 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -1732,18 +1732,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) { @@ -1766,30 +1754,6 @@ pg_stat_get_checkpoint_sync_time(PG_FUNCTION_ARGS) pgstat_fetch_stat_checkpointer()->checkpoint_sync_time); } -Datum -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 BUFFERS_NUM_COLUMNS. diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 9d3ab6d0a3..c028ca58f4 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5600,25 +5600,11 @@ 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', proparallel => 'r', prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_bgwriter_maxwritten_clean' }, -{ oid => '3075', descr => 'statistics: last reset for the bgwriter', - proname => 'pg_stat_get_bgwriter_stat_reset_time', provolatile => 's', - proparallel => 'r', prorettype => 'timestamptz', proargtypes => '', - prosrc => 'pg_stat_get_bgwriter_stat_reset_time' }, { oid => '3160', descr => 'statistics: checkpoint time spent writing buffers to disk, in milliseconds', proname => 'pg_stat_get_checkpoint_write_time', provolatile => 's', @@ -5629,18 +5615,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 through 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 caf5ef5678..1abdb4e7ba 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -522,9 +522,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; /* ---------- @@ -537,9 +535,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; @@ -970,10 +965,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; /* @@ -986,9 +978,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 fb17ed7f93..65634eef6d 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1799,13 +1799,25 @@ 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, + ( SELECT pg_stat_buffers.write + FROM pg_stat_buffers + WHERE ((pg_stat_buffers.backend_type = 'checkpointer'::text) AND (pg_stat_buffers.io_path = 'shared'::text))) AS buffers_checkpoint, + ( SELECT pg_stat_buffers.write + FROM pg_stat_buffers + WHERE ((pg_stat_buffers.backend_type = 'background writer'::text) AND (pg_stat_buffers.io_path = 'shared'::text))) 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; + ( SELECT pg_stat_buffers.write + FROM pg_stat_buffers + WHERE ((pg_stat_buffers.backend_type = 'client backend'::text) AND (pg_stat_buffers.io_path = 'shared'::text))) AS buffers_backend, + ( SELECT pg_stat_buffers.fsync + FROM pg_stat_buffers + WHERE ((pg_stat_buffers.backend_type = 'client backend'::text) AND (pg_stat_buffers.io_path = 'shared'::text))) AS buffers_backend_fsync, + ( SELECT pg_stat_buffers.alloc + FROM pg_stat_buffers + WHERE ((pg_stat_buffers.backend_type = 'client backend'::text) AND (pg_stat_buffers.io_path = 'shared'::text))) AS buffers_alloc, + ( SELECT pg_stat_buffers.stats_reset + FROM pg_stat_buffers + LIMIT 1) AS stats_reset; pg_stat_buffers| SELECT b.backend_type, b.io_path, b.alloc, -- 2.32.0