From 9a24c5d4dfd6657557f9a6e0f620ae21f3718862 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Fri, 6 Nov 2026 23:12:16 +0000 Subject: [PATCH v6 2/4] pgstat: Allow a kind to release its references after their flush A backend keeps a reference to each entry it uses until it exits or is asked to release references to dropped entries, and every such request makes every backend scan all its references. For a kind whose entries are dropped all the time, like pg_stat_statements evicting entries, the requests would be constant, the references to live entries would keep piling up, and dropped entries would stay allocated until their users flush anyway, as references with pending statistics cannot be released. Add a release_refs_after_flush option to PgStat_KindInfo, for custom variable-numbered kinds. A backend then releases its references to entries of the kind without pending statistics after flushing at a transaction boundary. As flushes happen at most once per second, longer than an entry may live under churn, it also flushes, without waiting, and releases the references to the entries it created at the end of the creating transaction. When no statistics are pending, and so no flush would release them, it releases its references at once. A lookup turns a reference to an entry dropped or reinitialized since into a new one, discarding its pending statistics. Dropped entries are then freed once their users report their statistics outside a transaction, with no requests. A long transaction, like a procedure committing in a loop, delays that until it ends. Only the references of kinds with the option are released, and when a kind has it, the other kinds only pay for checking it. Along the way, pgstat_flush_entry_ref() is split out of pgstat_flush_pending_entries() to also flush a single entry, the stale reference check of pgstat_gc_entry_refs() becomes pgstat_entry_ref_is_stale(), and pgstat_release_shared_ref() is split out of pgstat_release_entry_ref() to also turn a stale reference into a placeholder, without moving it out of the cache. There are no tests for the option yet. --- src/backend/utils/activity/pgstat.c | 130 ++++++++++++++++----- src/backend/utils/activity/pgstat_shmem.c | 133 ++++++++++++++++++++-- src/include/utils/pgstat_internal.h | 33 ++++++ 3 files changed, 262 insertions(+), 34 deletions(-) diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 1887538a5d0..68864426af7 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -218,6 +218,13 @@ PgStat_LocalState pgStatLocal; */ bool pgstat_report_fixed = false; +/* + * Does a kind use release_refs_after_flush? If not, releasing references + * is skipped entirely. Set by pgstat_register_kind(), so only custom kinds + * can use the option for now. + */ +bool pgstat_have_release_refs_kinds = false; + /* ---------- * Local data * @@ -784,10 +791,21 @@ pgstat_report_stat(bool force) pgStatForceNextFlush = false; } + /* + * Release the references to the entries created since stats were last + * reported outside a transaction, as they may be dropped before the next + * flush. Not once stats are shut down, as this flushes their statistics. + */ + if (!IsTransactionOrTransactionBlock() && !pgStatLocal.shmem->is_shutdown) + pgstat_release_created_entry_refs(); + /* Don't expend a clock check if nothing to do */ if (dlist_is_empty(&pgStatPending) && !pgstat_report_fixed) { + /* No flush will release the references without pending statistics */ + if (!IsTransactionOrTransactionBlock()) + pgstat_release_flushed_entry_refs(); return 0; } @@ -846,6 +864,13 @@ pgstat_report_stat(bool force) /* flush of variable-numbered stats tracked in pending entries list */ partial_flush |= pgstat_flush_pending_entries(nowait); + /* + * Release the other references of kinds that must not outlive the + * flush, now that their pending statistics are flushed. + */ + if (!IsTransactionOrTransactionBlock()) + pgstat_release_flushed_entry_refs(); + /* flush of other stats kinds */ if (pgstat_report_fixed) { @@ -1430,6 +1455,77 @@ pgstat_delete_pending_entry(PgStat_EntryRef *entry_ref) dlist_delete(&entry_ref->pending_node); } +/* + * Flush the pending statistics of an entry, if possible. The caller resets + * pgStatCurrentFlushEntry if this errors out. + */ +static PgStat_FlushResult +pgstat_flush_entry_ref(PgStat_EntryRef *entry_ref, bool nowait, + bool xact_boundary) +{ + const PgStat_KindInfo *kind_info = + pgstat_get_kind_info(entry_ref->shared_entry->key.kind); + PgStat_FlushResult result; + + Assert(!kind_info->fixed_amount); + Assert(kind_info->flush_pending_cb != NULL); + + pgStatCurrentFlushEntry = entry_ref; + result = kind_info->flush_pending_cb(entry_ref, nowait, xact_boundary); + pgStatCurrentFlushEntry = NULL; + + /* + * A lock conflict can only happen when we allowed the callback to give up + * without waiting for a lock, and a partial flush can only happen inside + * a transaction. + */ + Assert(result == PGSTAT_FLUSH_DONE || + (result == PGSTAT_FLUSH_LOCK_CONFLICT && nowait) || + (result == PGSTAT_FLUSH_PARTIAL && !xact_boundary)); + + /* + * Mark the entry visited, unless we could not flush it at all. If a later + * callback adds more data to this entry in the same pass, + * pgstat_prep_pending_from_entry_ref() then re-queues it to the tail so + * the new data is flushed again before the pass ends. + */ + if (result != PGSTAT_FLUSH_LOCK_CONFLICT) + entry_ref->flushed_generation = pgStatFlushGeneration; + + return result; +} + +/* + * Flush the pending statistics of an entry at a transaction boundary, and + * delete them if flushed. Returns whether they were. + */ +bool +pgstat_flush_pending_entry(PgStat_EntryRef *entry_ref, bool nowait) +{ + PgStat_FlushResult result; + + Assert(!IsTransactionOrTransactionBlock()); + Assert(!pgStatFlushInProgress); + + /* As in pgstat_report_stat(), clear the flush state if this errors out */ + pgStatFlushInProgress = true; + PG_TRY(); + { + result = pgstat_flush_entry_ref(entry_ref, nowait, true); + } + PG_FINALLY(); + { + pgStatCurrentFlushEntry = NULL; + pgStatFlushInProgress = false; + } + PG_END_TRY(); + + if (result != PGSTAT_FLUSH_DONE) + return false; + pgstat_delete_pending_entry(entry_ref); + return true; +} + /* * Prepare the given entry to receive pending stats, if not already done. */ @@ -1509,9 +1605,6 @@ pgstat_flush_pending_entries(bool nowait) { PgStat_EntryRef *entry_ref = dlist_container(PgStat_EntryRef, pending_node, cur); - PgStat_HashKey key = entry_ref->shared_entry->key; - PgStat_Kind kind = key.kind; - const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); PgStat_FlushResult result; bool xact_boundary; dlist_node *next; @@ -1520,31 +1613,8 @@ pgstat_flush_pending_entries(bool nowait) xact_boundary = !IsTransactionOrTransactionBlock(); - Assert(!kind_info->fixed_amount); - Assert(kind_info->flush_pending_cb != NULL); - /* flush the stats, if possible */ - pgStatCurrentFlushEntry = entry_ref; - result = kind_info->flush_pending_cb(entry_ref, nowait, xact_boundary); - pgStatCurrentFlushEntry = NULL; - - /* - * A lock conflict can only happen when we allowed the callback to - * give up without waiting for a lock, and a partial flush can only - * happen inside a transaction. - */ - Assert(result == PGSTAT_FLUSH_DONE || - (result == PGSTAT_FLUSH_LOCK_CONFLICT && nowait) || - (result == PGSTAT_FLUSH_PARTIAL && !xact_boundary)); - - /* - * Mark the entry visited, unless we could not flush it at all. If a - * later callback adds more data to this entry in the same pass, - * pgstat_prep_pending_from_entry_ref() then re-queues it to the tail - * so the new data is flushed again before the pass ends. - */ - if (result != PGSTAT_FLUSH_LOCK_CONFLICT) - entry_ref->flushed_generation = pgStatFlushGeneration; + result = pgstat_flush_entry_ref(entry_ref, nowait, xact_boundary); /* * Determine the next entry after the callback ran (it may have @@ -1699,6 +1769,10 @@ pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info) ereport(ERROR, (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), errhint("Custom cumulative statistics cannot use serialized names for fixed-numbered objects."))); + if (kind_info->release_refs_after_flush) + ereport(ERROR, + (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), + errhint("Custom cumulative statistics cannot release references after their flush for fixed-numbered objects."))); } else { @@ -1741,6 +1815,8 @@ pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info) /* Register it */ pgstat_kind_custom_infos[idx] = kind_info; + if (kind_info->release_refs_after_flush) + pgstat_have_release_refs_kinds = true; ereport(LOG, (errmsg("registered custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind))); diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 883b2f0b65f..ca66c66d567 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -50,6 +50,8 @@ static void pgstat_drop_database_and_contents(Oid dboid); static void pgstat_free_entry(PgStatShared_HashEntry *shent, dshash_seq_status *hstat); static void pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, bool discard_pending); +static void pgstat_release_shared_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, bool discard_pending); +static void pgstat_remember_created_entry(PgStat_EntryRef *entry_ref); static bool pgstat_need_entry_refs_gc(void); static void pgstat_gc_entry_refs(void); static void pgstat_release_all_entry_refs(bool discard_pending); @@ -98,6 +100,28 @@ static int pgStatSharedRefAge = 0; /* cache age of pgStatLocal.shmem */ static MemoryContext pgStatSharedRefContext = NULL; static MemoryContext pgStatEntryRefHashContext = NULL; +/* + * References to the entries of kinds with release_refs_after_flush that this + * backend created since its last transaction boundary, see + * pgstat_release_created_entry_refs(). + */ +static dlist_head pgStatCreatedRefs = DLIST_STATIC_INIT(pgStatCreatedRefs); + +/* + * Number of references this backend holds to entries of kinds with + * release_refs_after_flush, so that releasing them is skipped when there is + * none, see pgstat_release_flushed_entry_refs(). + */ +static int pgStatNumReleaseRefs = 0; + +/* Does this kind release its references after their flush? */ +static inline bool +pgstat_kind_releases_refs(PgStat_Kind kind) +{ + return pgstat_have_release_refs_kinds && + pgstat_get_kind_info(kind)->release_refs_after_flush; +} + /* ------------------------------------------------------------ * Public functions called from postmaster follow @@ -522,6 +546,8 @@ pgstat_acquire_entry_ref(PgStat_EntryRef *entry_ref, entry_ref->shared_stats = shheader; entry_ref->shared_entry = shhashent; entry_ref->generation = pg_atomic_read_u32(&shhashent->generation); + if (pgstat_kind_releases_refs(shhashent->key.kind)) + pgStatNumReleaseRefs++; /* * Complete the local reference before releasing the lock. Releasing an @@ -572,6 +598,7 @@ pgstat_get_entry_ref_cached(PgStat_HashKey key, PgStat_EntryRef **entry_ref_p) entry_ref->shared_stats = NULL; entry_ref->shared_entry = NULL; entry_ref->pending = NULL; + dlist_node_init(&entry_ref->created_node); found = false; } @@ -649,7 +676,23 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, * match here we can avoid taking locks / causing contention. */ if (pgstat_get_entry_ref_cached(key, &entry_ref)) - return entry_ref; + { + /* + * For a kind whose references do not outlive their flush, replace a + * reference to an entry dropped or reinitialized since it was + * obtained, discarding its pending statistics. Callers must thus not + * keep a pointer to pending statistics across a lookup of the same + * entry. + */ + if (!pgstat_kind_releases_refs(kind) || + !pgstat_entry_ref_is_stale(entry_ref)) + return entry_ref; + + /* Make it a placeholder, which the lookup below fills */ + pgstat_release_shared_ref(key, entry_ref, true); + entry_ref->shared_stats = NULL; + entry_ref->shared_entry = NULL; + } Assert(entry_ref != NULL); @@ -708,6 +751,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, { shheader = pgstat_init_entry(kind, shhashent, chunk); pgstat_acquire_entry_ref(entry_ref, hash, shhashent, shheader); + pgstat_remember_created_entry(entry_ref); if (created_entry != NULL) *created_entry = true; @@ -750,6 +794,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, */ shheader = pgstat_reinit_entry(kind, shhashent); pgstat_acquire_entry_ref(entry_ref, hash, shhashent, shheader); + pgstat_remember_created_entry(entry_ref); if (created_entry != NULL) *created_entry = true; @@ -773,10 +818,20 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, } } +/* + * Release the pending statistics of a reference and its reference to the + * shared entry, keeping it in the cache. + */ static void -pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, - bool discard_pending) +pgstat_release_shared_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, + bool discard_pending) { + if (entry_ref && !dlist_node_is_detached(&entry_ref->created_node)) + dlist_delete_thoroughly(&entry_ref->created_node); + if (entry_ref && entry_ref->shared_stats && + pgstat_kind_releases_refs(key.kind)) + pgStatNumReleaseRefs--; + if (entry_ref && entry_ref->pending) { if (discard_pending) @@ -822,6 +877,13 @@ pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, } } } +} + +static void +pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, + bool discard_pending) +{ + pgstat_release_shared_ref(key, entry_ref, discard_pending); if (!pgstat_entry_ref_hash_delete(pgStatEntryRefHash, key)) elog(ERROR, "entry ref vanished before deletion"); @@ -949,9 +1011,7 @@ pgstat_gc_entry_refs(void) * "generation" checks for the case of entries being reinitialized, * and "dropped" for the case where these are.. dropped. */ - if (!entry_ref->shared_entry->dropped && - pg_atomic_read_u32(&entry_ref->shared_entry->generation) == - entry_ref->generation) + if (!pgstat_entry_ref_is_stale(entry_ref)) continue; /* cannot gc shared ref that has pending data */ @@ -988,6 +1048,62 @@ pgstat_release_matching_entry_refs(bool discard_pending, ReleaseMatchCB match, } } +static bool +match_flushed_entry_ref(PgStat_EntryRefHashEntry *ent, Datum match_data) +{ + return ent->entry_ref->shared_stats != NULL && + ent->entry_ref->pending == NULL && + pgstat_kind_releases_refs(ent->key.kind); +} + +/* + * Release this backend's references to entries of kinds with + * release_refs_after_flush, except those with pending statistics. Called + * at a transaction boundary, after flushing pending statistics or when none + * are pending. + */ +void +pgstat_release_flushed_entry_refs(void) +{ + if (pgStatNumReleaseRefs > 0) + pgstat_release_matching_entry_refs(false, match_flushed_entry_ref, 0); +} + +/* + * Remember that this backend created the entry of "entry_ref", if its kind + * has release_refs_after_flush, see pgstat_release_created_entry_refs(). + */ +static void +pgstat_remember_created_entry(PgStat_EntryRef *entry_ref) +{ + if (pgstat_kind_releases_refs(entry_ref->shared_entry->key.kind)) + dlist_push_tail(&pgStatCreatedRefs, &entry_ref->created_node); +} + +/* + * Flush and release the references to the entries of kinds with + * release_refs_after_flush that this backend created in its last + * transaction, as they may be dropped before the next flush. A reference + * whose statistics cannot be flushed without waiting is left to that flush. + */ +void +pgstat_release_created_entry_refs(void) +{ + while (!dlist_is_empty(&pgStatCreatedRefs)) + { + PgStat_EntryRef *entry_ref = + dlist_head_element(PgStat_EntryRef, created_node, &pgStatCreatedRefs); + + /* Unlink first, as the flush callback may release references */ + dlist_delete_thoroughly(&entry_ref->created_node); + + if (entry_ref->pending != NULL && + !pgstat_flush_pending_entry(entry_ref, true)) + continue; + pgstat_release_entry_ref(entry_ref->shared_entry->key, entry_ref, false); + } +} + /* * Release all local references to shared stats entries. * @@ -1157,7 +1273,10 @@ pgstat_drop_database_and_contents(Oid dboid) * The callers of this function should call pgstat_request_entry_refs_gc() * if the stats entry could not be freed, to ensure that this entry's memory * can be reclaimed later by a different backend calling - * pgstat_gc_entry_refs(). + * pgstat_gc_entry_refs(). Kinds with release_refs_after_flush can skip + * this, as the references of other backends are released at their next + * report of statistics outside a transaction, which a long transaction only + * delays. */ bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 74e8bfd50e6..1310f0bab6c 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -194,6 +194,12 @@ typedef struct PgStat_EntryRef void *pending; dlist_node pending_node; /* membership in pgStatPending list */ + /* + * Membership in the list of the entries created in the current + * transaction, for kinds with release_refs_after_flush. + */ + dlist_node created_node; + /* * Flush-pass generation in which this entry was last flushed. Used by * pgstat_prep_pending_from_entry_ref() to decide whether an entry being @@ -295,6 +301,14 @@ typedef struct PgStat_KindInfo */ bool own_hash:1; + /* + * Release references to entries of this kind after their flush, and to + * entries created at the end of the creating transaction? For kinds + * whose entries are dropped often, so that they are freed without asking + * all backends to release their references. + */ + bool release_refs_after_flush:1; + /* * The size of an entry pointed to by PgStatShared_HashEntry->body. For * fixed-numbered statistics, this is the size of an entry in @@ -739,6 +753,7 @@ static inline size_t pgstat_get_entry_len(PgStat_Kind kind); static inline void *pgstat_get_entry_data(PgStat_Kind kind, PgStatShared_Common *entry); static inline void *pgstat_get_custom_shmem_data(PgStat_Kind kind); static inline void *pgstat_get_custom_snapshot_data(PgStat_Kind kind); +static inline bool pgstat_entry_ref_is_stale(PgStat_EntryRef *entry_ref); /* @@ -756,6 +771,7 @@ extern void pgstat_assert_is_up(void); #endif extern void pgstat_delete_pending_entry(PgStat_EntryRef *entry_ref); +extern bool pgstat_flush_pending_entry(PgStat_EntryRef *entry_ref, bool nowait); extern void pgstat_prep_pending_from_entry_ref(PgStat_EntryRef *entry_ref); extern PgStat_EntryRef *pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, @@ -913,6 +929,8 @@ extern void pgstat_reset_matching_entries(bool (*do_reset) (PgStatShared_HashEnt TimestampTz ts); extern void pgstat_request_entry_refs_gc(void); +extern void pgstat_release_flushed_entry_refs(void); +extern void pgstat_release_created_entry_refs(void); extern dsa_pointer pgstat_alloc_entry_body(PgStat_Kind kind); extern PgStatShared_Common *pgstat_init_entry(PgStat_Kind kind, PgStatShared_HashEntry *shhashent, @@ -977,6 +995,9 @@ extern void pgstat_create_transactional(PgStat_Kind kind, Oid dboid, uint64 obji */ extern PGDLLIMPORT bool pgstat_report_fixed; +/* Is there a kind with release_refs_after_flush? */ +extern PGDLLIMPORT bool pgstat_have_release_refs_kinds; + /* Backend-local stats state */ extern PGDLLIMPORT PgStat_LocalState pgStatLocal; @@ -1093,6 +1114,18 @@ pgstat_get_entry_len(PgStat_Kind kind) return pgstat_get_kind_info(kind)->shared_data_len; } +/* + * Has the entry of a reference been dropped, or reinitialized, since the + * reference was obtained? + */ +static inline bool +pgstat_entry_ref_is_stale(PgStat_EntryRef *entry_ref) +{ + return entry_ref->shared_entry->dropped || + pg_atomic_read_u32(&entry_ref->shared_entry->generation) != + entry_ref->generation; +} + /* * Returns a pointer to the data portion of a shared memory stats entry. */ -- 2.50.1