From baae117915069f3807c67c98b649dbeab71fc47c Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 24 Nov 2021 12:20:10 -0500 Subject: [PATCH v18 7/8] Remove superfluous bgwriter stats Remove stats from pg_stat_bgwriter which are now more clearly expressed in pg_stat_buffers. Rename pg_stat_bgwriter to pg_stat_checkpointer since most of the stats now concern checkpointer. TODO: - make pg_stat_bgwriter view again and move maxwritten_clean into it - add additional stats to pg_stat_bgwriter --- doc/src/sgml/monitoring.sgml | 69 +++++---------------------- src/backend/catalog/system_views.sql | 11 ++--- src/backend/postmaster/checkpointer.c | 29 +---------- src/backend/postmaster/pgstat.c | 13 ++--- src/backend/storage/buffer/bufmgr.c | 6 --- src/backend/utils/adt/pgstatfuncs.c | 34 +------------ src/include/catalog/pg_proc.dat | 34 +++---------- src/include/pgstat.h | 12 +---- src/test/regress/expected/rules.out | 17 +++---- 9 files changed, 35 insertions(+), 190 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index b16952d439..412f0d2502 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -427,11 +427,11 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser - pg_stat_bgwriterpg_stat_bgwriter + pg_stat_checkpointerpg_stat_checkpointer One row only, showing statistics about the background writer process's activity. See - - pg_stat_bgwriter for details. + + pg_stat_checkpointer for details. @@ -3485,20 +3485,20 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i - - <structname>pg_stat_bgwriter</structname> + + <structname>pg_stat_checkpointer</structname> - pg_stat_bgwriter + pg_stat_checkpointer - The pg_stat_bgwriter view will always have a + The pg_stat_checkpointer view will always have a single row, containing global data for the cluster. - - <structname>pg_stat_bgwriter</structname> View +
+ <structname>pg_stat_checkpointer</structname> View @@ -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 @@ -5313,9 +5266,9 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i Resets some cluster-wide statistics counters to zero, depending on the - argument. The argument can be bgwriter to reset + argument. The argument can be checkpointer to reset all the counters shown in - the pg_stat_bgwriter + the pg_stat_checkpointer view, archiver to reset all the counters shown in the pg_stat_archiver view, wal to reset all the counters shown in the pg_stat_wal view, diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index e214d23056..6258c2770c 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1062,18 +1062,13 @@ CREATE VIEW pg_stat_archiver AS s.stats_reset FROM pg_stat_get_archiver() s; -CREATE VIEW pg_stat_bgwriter AS +CREATE VIEW pg_stat_checkpointer AS SELECT - pg_stat_get_bgwriter_timed_checkpoints() AS checkpoints_timed, - pg_stat_get_bgwriter_requested_checkpoints() AS checkpoints_req, + pg_stat_get_timed_checkpoints() AS checkpoints_timed, + pg_stat_get_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 3545e197ce..13e46f497f 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -1601,8 +1601,8 @@ pgstat_reset_shared_counters(const char *target) } else if (strcmp(target, "archiver") == 0) msg.m_resettarget = RESET_ARCHIVER; - else if (strcmp(target, "bgwriter") == 0) - msg.m_resettarget = RESET_BGWRITER; + else if (strcmp(target, "checkpointer") == 0) + msg.m_resettarget = RESET_CHECKPOINTER; else if (strcmp(target, "wal") == 0) msg.m_resettarget = RESET_WAL; else @@ -1610,7 +1610,7 @@ pgstat_reset_shared_counters(const char *target) (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("unrecognized reset target: \"%s\"", target), errhint( - "Target must be \"archiver\", \"bgwriter\", \"buffers\", or \"wal\"."))); + "Target must be \"archiver\", \"checkpointer\", \"buffers\", or \"wal\"."))); pgstat_send(&msg, sizeof(msg)); @@ -5679,7 +5679,7 @@ pgstat_recv_resetcounter(PgStat_MsgResetcounter *msg, int len) static void pgstat_recv_resetsharedcounter(PgStat_MsgResetsharedcounter *msg, int len) { - if (msg->m_resettarget == RESET_BGWRITER) + if (msg->m_resettarget == RESET_CHECKPOINTER) { /* * Reset the global bgwriter and checkpointer statistics for the @@ -5985,9 +5985,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; } /* ---------- @@ -6003,9 +6001,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 949f1088b6..8120567de1 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 26730230b6..52500c8af4 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -1727,29 +1727,17 @@ pg_stat_get_db_sessions_killed(PG_FUNCTION_ARGS) } Datum -pg_stat_get_bgwriter_timed_checkpoints(PG_FUNCTION_ARGS) +pg_stat_get_timed_checkpoints(PG_FUNCTION_ARGS) { PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->timed_checkpoints); } Datum -pg_stat_get_bgwriter_requested_checkpoints(PG_FUNCTION_ARGS) +pg_stat_get_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 BUFFERS_NUM_COLUMNS. diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 148208d242..293df23040 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5585,25 +5585,15 @@ proargnames => '{archived_count,last_archived_wal,last_archived_time,failed_count,last_failed_wal,last_failed_time,stats_reset}', prosrc => 'pg_stat_get_archiver' }, { oid => '2769', - descr => 'statistics: number of timed checkpoints started by the bgwriter', - proname => 'pg_stat_get_bgwriter_timed_checkpoints', provolatile => 's', + descr => 'statistics: number of scheduled checkpoints performed', + proname => 'pg_stat_get_timed_checkpoints', provolatile => 's', proparallel => 'r', prorettype => 'int8', proargtypes => '', - prosrc => 'pg_stat_get_bgwriter_timed_checkpoints' }, + prosrc => 'pg_stat_get_timed_checkpoints' }, { oid => '2770', - descr => 'statistics: number of backend requested checkpoints started by the bgwriter', - proname => 'pg_stat_get_bgwriter_requested_checkpoints', provolatile => 's', + descr => 'statistics: number of backend requested checkpoints performed', + proname => 'pg_stat_get_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' }, + prosrc => 'pg_stat_get_requested_checkpoints' }, { 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', @@ -5623,18 +5613,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 8e07584767..020cd4ca87 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -141,7 +141,7 @@ typedef struct PgStat_TableCounts typedef enum PgStat_Shared_Reset_Target { RESET_ARCHIVER, - RESET_BGWRITER, + RESET_CHECKPOINTER, RESET_BUFFERS, RESET_WAL } PgStat_Shared_Reset_Target; @@ -523,9 +523,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; /* ---------- @@ -538,9 +536,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; @@ -971,9 +966,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; @@ -987,9 +980,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..13f48722c3 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1817,17 +1817,6 @@ pg_stat_archiver| SELECT s.archived_count, s.last_failed_time, s.stats_reset FROM pg_stat_get_archiver() s(archived_count, last_archived_wal, last_archived_time, failed_count, last_failed_wal, last_failed_time, stats_reset); -pg_stat_bgwriter| 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; pg_stat_buffers| SELECT b.backend_type, b.io_path, b.alloc, @@ -1836,6 +1825,12 @@ pg_stat_buffers| SELECT b.backend_type, b.write, b.stats_reset FROM pg_stat_get_buffers() b(backend_type, io_path, alloc, extend, fsync, write, stats_reset); +pg_stat_checkpointer| SELECT pg_stat_get_timed_checkpoints() AS checkpoints_timed, + pg_stat_get_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_maxwritten_clean() AS maxwritten_clean, + pg_stat_get_bgwriter_stat_reset_time() AS stats_reset; pg_stat_database| SELECT d.oid AS datid, d.datname, CASE -- 2.30.2