From 8c7fd33e98ec958308979dba9cb7897787ddc48e Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Tue, 17 Dec 2024 15:15:23 +0900 Subject: [PATCH v9 2/2] Tweaks on top of v9 Other notes for commit: Requires a bump of CATALOG_VERSION_NO. Requires a bump of PGSTAT_FILE_FORMAT_ID. --- src/include/catalog/pg_proc.dat | 3 +- src/include/pgstat.h | 11 +++-- src/backend/utils/activity/backend_status.c | 2 +- src/backend/utils/activity/pgstat_backend.c | 38 ++++++++------- src/backend/utils/activity/pgstat_io.c | 2 - src/backend/utils/adt/pgstatfuncs.c | 53 +++++++++------------ src/test/regress/expected/stats.out | 28 +++++++---- src/test/regress/sql/stats.sql | 12 +++-- doc/src/sgml/monitoring.sgml | 10 ++-- 9 files changed, 86 insertions(+), 73 deletions(-) diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 437157ffa3..2dcc2d42da 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6061,8 +6061,7 @@ proname => 'pg_stat_reset_single_function_counters', provolatile => 'v', prorettype => 'void', proargtypes => 'oid', prosrc => 'pg_stat_reset_single_function_counters' }, -{ oid => '9987', - descr => 'statistics: reset statistics for a single backend', +{ oid => '8807', descr => 'statistics: reset statistics for a single backend', proname => 'pg_stat_reset_backend_stats', provolatile => 'v', prorettype => 'void', proargtypes => 'int4', prosrc => 'pg_stat_reset_backend_stats' }, diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 479773cfd2..56ed1b8bb3 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -363,11 +363,11 @@ typedef struct PgStat_BktypeIO PgStat_Counter times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES]; } PgStat_BktypeIO; -typedef struct PgStat_BackendPendingIO +typedef struct PgStat_PendingIO { PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES]; instr_time pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES]; -} PgStat_BackendPendingIO; +} PgStat_PendingIO; typedef struct PgStat_IO { @@ -375,6 +375,9 @@ typedef struct PgStat_IO PgStat_BktypeIO stats[BACKEND_NUM_TYPES]; } PgStat_IO; +/* Backend statistics store the same amount of IO data as PGSTAT_KIND_IO */ +typedef PgStat_PendingIO PgStat_BackendPendingIO; + typedef struct PgStat_Backend { TimestampTz stat_reset_timestamp; @@ -565,9 +568,9 @@ extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void); * Functions in pgstat_backend.c */ -extern PgStat_Backend *pgstat_fetch_proc_stat_io(ProcNumber procNumber); +extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber); extern bool pgstat_tracks_backend_bktype(BackendType bktype); -extern void pgstat_create_backend_stat(ProcNumber procnum); +extern void pgstat_create_backend(ProcNumber procnum); /* * Functions in pgstat_bgwriter.c diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index e9c3b87ac2..bf33e33a4e 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -428,7 +428,7 @@ pgstat_bestart(void) /* Create the backend statistics entry */ if (pgstat_tracks_backend_bktype(MyBackendType)) - pgstat_create_backend_stat(MyProcNumber); + pgstat_create_backend(MyProcNumber); /* Update app name to current GUC setting */ if (application_name) diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index e2d83024c2..6b2c9baa8c 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -3,11 +3,17 @@ * pgstat_backend.c * Implementation of backend statistics. * - * This file contains the implementation of backend statistics. It is kept + * This file contains the implementation of backend statistics. It is kept * separate from pgstat.c to enforce the line between the statistics access / - * storage implementation and the details about individual types of statistics. + * storage implementation and the details about individual types of + * statistics. * - * Copyright (c) 2024, PostgreSQL Global Development Group + * This statistics kind uses a proc number as object ID for the hash table + * of pgstats. Entries are created each time a process is spawned, and are + * dropped when the process exits. These are not written to the pgstats file + * on disk. + * + * Copyright (c) 2001-2024, PostgreSQL Global Development Group * * IDENTIFICATION * src/backend/utils/activity/pgstat_backend.c @@ -19,10 +25,10 @@ #include "utils/pgstat_internal.h" /* - * Returns backend's IO stats. + * Returns statistics of a backend by proc number. */ PgStat_Backend * -pgstat_fetch_proc_stat_io(ProcNumber procNumber) +pgstat_fetch_stat_backend(ProcNumber procNumber) { PgStat_Backend *backend_entry; @@ -81,21 +87,21 @@ pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait) void pgstat_flush_backend(bool nowait) { - if (pgstat_tracks_backend_bktype(MyBackendType)) - { - PgStat_EntryRef *entry_ref; + PgStat_EntryRef *entry_ref; - entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid, - MyProcNumber, false, NULL); - (void) pgstat_backend_flush_cb(entry_ref, nowait); - } + if (!pgstat_tracks_backend_bktype(MyBackendType)) + return; + + entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid, + MyProcNumber, false, NULL); + (void) pgstat_backend_flush_cb(entry_ref, nowait); } /* - * Create the backend statistics entry for procnum. + * Create backend statistics entry for proc number. */ void -pgstat_create_backend_stat(ProcNumber procnum) +pgstat_create_backend(ProcNumber procnum) { PgStat_EntryRef *entry_ref; PgStatShared_Backend *shstatent; @@ -113,7 +119,7 @@ pgstat_create_backend_stat(ProcNumber procnum) } /* - * Find or create a local PgStat_BackendPendingIO entry for procnum. + * Find or create a local PgStat_BackendPendingIO entry for proc number. */ PgStat_BackendPendingIO * pgstat_prep_backend_pending(ProcNumber procnum) @@ -136,7 +142,7 @@ pgstat_prep_backend_pending(ProcNumber procnum) * I/O stats are already visible in pg_stat_io and there is only one of those. * * Function returns true if BackendType participates in the backend stats - * subsystem for IO and false if it does not. + * subsystem and false if it does not. * * When adding a new BackendType, also consider adding relevant restrictions to * pgstat_tracks_io_object() and pgstat_tracks_io_op(). diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index e0c206a453..011a3326da 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -20,8 +20,6 @@ #include "storage/bufmgr.h" #include "utils/pgstat_internal.h" -typedef PgStat_BackendPendingIO PgStat_PendingIO; - static PgStat_PendingIO PendingIOStats; static bool have_iostats = false; diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index b939551d36..640e687604 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -1474,20 +1474,22 @@ pg_stat_get_io(PG_FUNCTION_ARGS) return (Datum) 0; } +/* + * Returns I/O statistics for a backend with given PID. + */ Datum pg_stat_get_backend_io(PG_FUNCTION_ARGS) { ReturnSetInfo *rsinfo; - PgStat_Backend *backend_stats; Datum bktype_desc; - PgStat_BktypeIO *bktype_stats; BackendType bktype; Datum reset_time; - int num_backends = pgstat_fetch_stat_numbackends(); - int curr_backend; int pid; PGPROC *proc; ProcNumber procNumber; + PgStat_Backend *backend_stats; + PgStat_BktypeIO *bktype_stats; + PgBackendStatus *beentry; InitMaterializedSRF(fcinfo, 0); rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; @@ -1496,40 +1498,28 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS) proc = BackendPidGetProc(pid); /* - * Could be an auxiliary process but would not report any stats due to - * pgstat_tracks_backend_bktype() anyway. So don't need an extra call to - * AuxiliaryPidGetProc(). + * This could be an auxiliary process but these do not report backend + * statistics due to pgstat_tracks_backend_bktype(), so there is no need + * for an extra call to AuxiliaryPidGetProc(). */ if (!proc) return (Datum) 0; procNumber = GetNumberFromPGProc(proc); - backend_stats = pgstat_fetch_proc_stat_io(procNumber); + + backend_stats = pgstat_fetch_stat_backend(procNumber); if (!backend_stats) return (Datum) 0; - bktype = B_INVALID; + beentry = pgstat_get_beentry_by_proc_number(procNumber); + bktype = beentry->st_backendType; - /* Look for the backend type */ - for (curr_backend = 1; curr_backend <= num_backends; curr_backend++) - { - LocalPgBackendStatus *local_beentry; - PgBackendStatus *beentry; + /* If PID does not match, leave */ + if (beentry->st_procpid != pid) + return (Datum) 0; - /* Get the next one in the list */ - local_beentry = pgstat_get_local_beentry_by_index(curr_backend); - beentry = &local_beentry->backendStatus; - - /* Looking for specific PID, ignore all the others */ - if (beentry->st_procpid != pid) - continue; - - bktype = beentry->st_backendType; - break; - } - - /* Backend is gone */ + /* Backend may be gone, so recheck in case */ if (bktype == B_INVALID) return (Datum) 0; @@ -1927,6 +1917,9 @@ pg_stat_reset_single_function_counters(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* + * Reset statistics of backend with given PID. + */ Datum pg_stat_reset_backend_stats(PG_FUNCTION_ARGS) { @@ -1936,9 +1929,9 @@ pg_stat_reset_backend_stats(PG_FUNCTION_ARGS) proc = BackendPidGetProc(backend_pid); /* - * Could be an auxiliary process but would not report any stats due to - * pgstat_tracks_backend_bktype() anyway. So don't need an extra call to - * AuxiliaryPidGetProc(). + * This could be an auxiliary process but these do not report backend + * statistics due to pgstat_tracks_backend_bktype(), so there is no need + * for an extra call to AuxiliaryPidGetProc(). */ if (!proc) PG_RETURN_VOID(); diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out index 3447e7b75d..150b6dcf74 100644 --- a/src/test/regress/expected/stats.out +++ b/src/test/regress/expected/stats.out @@ -1249,7 +1249,8 @@ SELECT pg_stat_get_subscription_stats(NULL); (1 row) --- Test that the following operations are tracked in pg_stat_io and in backend stats: +-- Test that the following operations are tracked in pg_stat_io and in +-- backend stats: -- - reads of target blocks into shared buffers -- - writes of shared buffers to permanent storage -- - extends of relations using shared buffers @@ -1335,14 +1336,6 @@ SELECT current_setting('fsync') = 'off' t (1 row) --- Don't return any rows if querying other backend's stats that are excluded --- from the backend stats collection (like the checkpointer). -SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid); - ?column? ----------- - t -(1 row) - -- Change the tablespace so that the table is rewritten directly, then SELECT -- from it to cause it to be read back into shared buffers. SELECT sum(reads) AS io_sum_shared_before_reads @@ -1603,6 +1596,23 @@ SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset; t (1 row) +-- Check invalid input for pg_stat_get_backend_io() +SELECT pg_stat_get_backend_io(NULL); + pg_stat_get_backend_io +------------------------ +(0 rows) + +SELECT pg_stat_get_backend_io(0); + pg_stat_get_backend_io +------------------------ +(0 rows) + +-- Auxiliary processes return no data. +SELECT pg_stat_get_backend_io(:checkpointer_pid); + pg_stat_get_backend_io +------------------------ +(0 rows) + -- test BRIN index doesn't block HOT update CREATE TABLE brin_hot ( id integer PRIMARY KEY, diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql index 9c925005be..1e7d0ff665 100644 --- a/src/test/regress/sql/stats.sql +++ b/src/test/regress/sql/stats.sql @@ -595,7 +595,8 @@ SELECT pg_stat_get_replication_slot(NULL); SELECT pg_stat_get_subscription_stats(NULL); --- Test that the following operations are tracked in pg_stat_io and in backend stats: +-- Test that the following operations are tracked in pg_stat_io and in +-- backend stats: -- - reads of target blocks into shared buffers -- - writes of shared buffers to permanent storage -- - extends of relations using shared buffers @@ -650,10 +651,6 @@ SELECT current_setting('fsync') = 'off' OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs AND :my_io_sum_shared_after_fsyncs= 0); --- Don't return any rows if querying other backend's stats that are excluded --- from the backend stats collection (like the checkpointer). -SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid); - -- Change the tablespace so that the table is rewritten directly, then SELECT -- from it to cause it to be read back into shared buffers. SELECT sum(reads) AS io_sum_shared_before_reads @@ -801,6 +798,11 @@ SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + FROM pg_stat_get_backend_io(pg_backend_pid()) \gset SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset; +-- Check invalid input for pg_stat_get_backend_io() +SELECT pg_stat_get_backend_io(NULL); +SELECT pg_stat_get_backend_io(0); +-- Auxiliary processes return no data. +SELECT pg_stat_get_backend_io(:checkpointer_pid); -- test BRIN index doesn't block HOT update CREATE TABLE brin_hot ( diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 4a9464da61..d0d176cc54 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -4801,11 +4801,13 @@ description | Waiting for a newly initialized WAL file to reach durable storage Returns I/O statistics about the backend with the specified process ID. The output fields are exactly the same as the ones in the - pg_stat_io - view. The function does not return I/O statistics for the checkpointer, + pg_stat_io view. + + + The function does not return I/O statistics for the checkpointer, the background writer, the startup process and the autovacuum launcher - as they are already visible in the pg_stat_io - view and there is only one of those. + as they are already visible in the pg_stat_io + view and there is only one of each. -- 2.45.2