commit af786645e63b34e0d4eefbef7a8dd0794d358d0f Author: Bertrand Drouvot Date: Mon Jan 20 09:57:26 2025 +0000 Change flush callbacks names and the way pending backend stats are checks and set to zero. diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index bd1ffad6f46..0a4b687f045 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -292,7 +292,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_len = sizeof(((PgStatShared_Database *) 0)->stats), .pending_size = sizeof(PgStat_StatDBEntry), - .flush_pending_cb = pgstat_database_flush_cb, + .flush_dynamic_cb = pgstat_database_flush_cb, .reset_timestamp_cb = pgstat_database_reset_timestamp_cb, }, @@ -307,7 +307,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_len = sizeof(((PgStatShared_Relation *) 0)->stats), .pending_size = sizeof(PgStat_TableStatus), - .flush_pending_cb = pgstat_relation_flush_cb, + .flush_dynamic_cb = pgstat_relation_flush_cb, .delete_pending_cb = pgstat_relation_delete_pending_cb, }, @@ -322,7 +322,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_len = sizeof(((PgStatShared_Function *) 0)->stats), .pending_size = sizeof(PgStat_FunctionCounts), - .flush_pending_cb = pgstat_function_flush_cb, + .flush_dynamic_cb = pgstat_function_flush_cb, }, [PGSTAT_KIND_REPLSLOT] = { @@ -355,7 +355,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_len = sizeof(((PgStatShared_Subscription *) 0)->stats), .pending_size = sizeof(PgStat_BackendSubEntry), - .flush_pending_cb = pgstat_subscription_flush_cb, + .flush_dynamic_cb = pgstat_subscription_flush_cb, .reset_timestamp_cb = pgstat_subscription_reset_timestamp_cb, }, @@ -373,7 +373,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .pending_size = sizeof(PgStat_BackendPending), .have_pending_cb = pgstat_backend_have_pending_cb, - .flush_cb = pgstat_backend_flush_cb, + .flush_static_cb = pgstat_backend_flush_cb, .reset_timestamp_cb = pgstat_backend_reset_timestamp_cb, }, @@ -438,7 +438,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_off = offsetof(PgStatShared_IO, stats), .shared_data_len = sizeof(((PgStatShared_IO *) 0)->stats), - .flush_cb = pgstat_io_flush_cb, + .flush_static_cb = pgstat_io_flush_cb, .have_pending_cb = pgstat_io_have_pending_cb, .init_shmem_cb = pgstat_io_init_shmem_cb, .reset_all_cb = pgstat_io_reset_all_cb, @@ -456,7 +456,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_off = offsetof(PgStatShared_SLRU, stats), .shared_data_len = sizeof(((PgStatShared_SLRU *) 0)->stats), - .flush_cb = pgstat_slru_flush_cb, + .flush_static_cb = pgstat_slru_flush_cb, .have_pending_cb = pgstat_slru_have_pending_cb, .init_shmem_cb = pgstat_slru_init_shmem_cb, .reset_all_cb = pgstat_slru_reset_all_cb, @@ -475,7 +475,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE] .shared_data_len = sizeof(((PgStatShared_Wal *) 0)->stats), .init_backend_cb = pgstat_wal_init_backend_cb, - .flush_cb = pgstat_wal_flush_cb, + .flush_static_cb = pgstat_wal_flush_cb, .have_pending_cb = pgstat_wal_have_pending_cb, .init_shmem_cb = pgstat_wal_init_shmem_cb, .reset_all_cb = pgstat_wal_reset_all_cb, @@ -785,23 +785,20 @@ pgstat_report_stat(bool force) partial_flush = false; - /* flush of variable-numbered stats */ + /* flush of variable-numbered stats tracked in pending entries list */ partial_flush |= pgstat_flush_pending_entries(nowait); - /* - * Flush of other stats, which could be variable-numbered or - * fixed-numbered. - */ + /* flush stats for each registered kind that has a flush static callback */ for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); if (!kind_info) continue; - if (!kind_info->flush_cb) + if (!kind_info->flush_static_cb) continue; - partial_flush |= kind_info->flush_cb(nowait); + partial_flush |= kind_info->flush_static_cb(nowait); } last_flush = now; @@ -1291,7 +1288,7 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *creat PgStat_EntryRef *entry_ref; /* need to be able to flush out */ - Assert(pgstat_get_kind_info(kind)->flush_pending_cb != NULL); + Assert(pgstat_get_kind_info(kind)->flush_dynamic_cb != NULL); if (unlikely(!pgStatPendingContext)) { @@ -1388,10 +1385,10 @@ pgstat_flush_pending_entries(bool nowait) dlist_node *next; Assert(!kind_info->fixed_amount); - Assert(kind_info->flush_pending_cb != NULL); + Assert(kind_info->flush_dynamic_cb != NULL); /* flush the stats, if possible */ - did_flush = kind_info->flush_pending_cb(entry_ref, nowait); + did_flush = kind_info->flush_dynamic_cb(entry_ref, nowait); Assert(did_flush || nowait); diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c index fb7abf64a0b..bcae6a78169 100644 --- a/src/backend/utils/activity/pgstat_backend.c +++ b/src/backend/utils/activity/pgstat_backend.c @@ -29,12 +29,11 @@ #include "utils/pgstat_internal.h" /* - * Backend statistics counts waiting to be flushed out. We assume this variable - * inits to zeroes. These counters may be reported within critical sections so - * we use static memory in order to avoid memory allocation. + * Backend statistics counts waiting to be flushed out. These counters may be + * reported within critical sections so we use static memory in order to avoid + * memory allocation. */ -static PgStat_BackendPending PendingBackendStats = {0}; -static bool have_backendstats = false; +static PgStat_BackendPending PendingBackendStats; /* * Utility routines to report I/O stats for backends, kept here to avoid @@ -53,8 +52,6 @@ pgstat_count_backend_io_op_time(IOObject io_object, IOContext io_context, INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op], io_time); - - have_backendstats = true; } void @@ -68,8 +65,6 @@ pgstat_count_backend_io_op(IOObject io_object, IOContext io_context, PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt; PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes; - - have_backendstats = true; } /* @@ -98,8 +93,8 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref) PgStat_PendingIO pending_io; /* - * This function can be called even if nothing at all has happened for - * IO statistics. In this case, avoid unnecessarily modifying the stats + * This function can be called even if nothing at all has happened for IO + * statistics. In this case, avoid unnecessarily modifying the stats * entry. */ if (pg_memory_is_all_zeros(&PendingBackendStats.pending_io, @@ -129,6 +124,11 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref) } } } + + /* + * Clear out the statistics buffer, so it can be re-used. + */ + MemSet(&PendingBackendStats.pending_io, 0, sizeof(PgStat_PendingIO)); } /* @@ -142,7 +142,8 @@ pgstat_flush_backend(bool nowait, bits32 flags) { PgStat_EntryRef *entry_ref; - if (!have_backendstats) + if (pg_memory_is_all_zeros(&PendingBackendStats, + sizeof(struct PgStat_BackendPending))) return false; if (!pgstat_tracks_backend_bktype(MyBackendType)) @@ -159,12 +160,6 @@ pgstat_flush_backend(bool nowait, bits32 flags) pgstat_unlock_entry(entry_ref); - /* - * Clear out the statistics buffer, so it can be re-used. - */ - MemSet(&PendingBackendStats, 0, sizeof(PgStat_BackendPending)); - - have_backendstats = false; return false; } @@ -174,7 +169,8 @@ pgstat_flush_backend(bool nowait, bits32 flags) bool pgstat_backend_have_pending_cb(void) { - return have_backendstats; + return (!pg_memory_is_all_zeros(&PendingBackendStats, + sizeof(struct PgStat_BackendPending))); } /* @@ -209,7 +205,6 @@ pgstat_create_backend(ProcNumber procnum) pgstat_unlock_entry(entry_ref); MemSet(&PendingBackendStats, 0, sizeof(PgStat_BackendPending)); - have_backendstats = false; } /* diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 000ed5b36f6..7222b414779 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -156,8 +156,8 @@ typedef struct PgStat_EntryRef * Pending statistics data that will need to be flushed to shared memory * stats eventually. Each stats kind utilizing pending data defines what * format its pending data has and needs to provide a - * PgStat_KindInfo->flush_pending_cb callback to merge pending into shared - * stats. + * PgStat_KindInfo->flush_dynamic_cb callback to merge pending entries + * that are in dynamic memory into shared stats. */ void *pending; dlist_node pending_node; /* membership in pgStatPending list */ @@ -259,10 +259,11 @@ typedef struct PgStat_KindInfo void (*init_backend_cb) (void); /* - * For variable-numbered stats: flush pending stats within the dshash. - * Required if pending data interacts with the pgstats dshash. + * For variable-numbered stats: flush pending stats entries in dynamic + * memory within the dshash. Required if pending data interacts with the + * pgstats dshash. */ - bool (*flush_pending_cb) (PgStat_EntryRef *sr, bool nowait); + bool (*flush_dynamic_cb) (PgStat_EntryRef *sr, bool nowait); /* * For variable-numbered stats: delete pending stats. Optional. @@ -298,10 +299,10 @@ typedef struct PgStat_KindInfo /* * For fixed-numbered or variable-numbered statistics: Flush pending - * stats. Returns true if some of the stats could not be flushed, due - * to lock contention for example. Optional. + * static stats. Returns true if some of the stats could not be flushed, + * due to lock contention for example. Optional. */ - bool (*flush_cb) (bool nowait); + bool (*flush_static_cb) (bool nowait); /* * For fixed-numbered statistics: Reset All. diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c index 5db62bca66f..4f3691c702b 100644 --- a/src/test/modules/injection_points/injection_stats.c +++ b/src/test/modules/injection_points/injection_stats.c @@ -48,7 +48,7 @@ static const PgStat_KindInfo injection_stats = { .shared_data_off = offsetof(PgStatShared_InjectionPoint, stats), .shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats), .pending_size = sizeof(PgStat_StatInjEntry), - .flush_pending_cb = injection_stats_flush_cb, + .flush_dynamic_cb = injection_stats_flush_cb, }; /*