From eb6950c7e19607bde8d5bae62c870d23bc7f39f2 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Thu, 11 Jun 2026 07:30:38 -0500 Subject: [PATCH v4 2/4] pgstat: allow a stats kind to use its own dedicated dshash The pgstat infrastructure stores all variable-numbered stats entries in a single shared dshash table. Any operation that needs entries of one specific kind must iterate past all entries of every other kind. Stats can now set a new varilable-numbered stats option own_hash to get a dedicated dshash table. Lookups and inserts are routed to the correct hash via pgstat_get_hash_for_kind(). Only variable-numbered stats kinds can opt in, since fixed-amount kinds have their storage pre-allocated at startup and their stats are not stored with other kinds in a dshash. The test_custom_var_stats module is updated to also test own_hash=true. --- src/backend/utils/activity/pgstat.c | 218 +++++++++--------- src/backend/utils/activity/pgstat_shmem.c | 186 ++++++++++----- src/include/utils/pgstat_internal.h | 29 +++ .../test_custom_stats/t/001_custom_stats.pl | 158 +++++++------ .../test_custom_var_stats--1.0.sql | 10 + .../test_custom_stats/test_custom_var_stats.c | 103 ++++++++- 6 files changed, 468 insertions(+), 236 deletions(-) diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 83f7cc819c9..f9123a081f1 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -1186,52 +1186,55 @@ pgstat_build_snapshot(void) /* * Snapshot all variable stats. */ - dshash_seq_init(&hstat, pgStatLocal.shared_hash, false); - while ((p = dshash_seq_next(&hstat)) != NULL) + for (int h = 0; h < pgStatLocal.num_hashes; h++) { - PgStat_Kind kind = p->key.kind; - const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); - bool found; - PgStat_SnapshotEntry *entry; - PgStatShared_Common *stats_data; - - /* - * Check if the stats object should be included in the snapshot. - * Unless the stats kind can be accessed from all databases (e.g., - * database stats themselves), we only include stats for the current - * database or objects not associated with a database (e.g. shared - * relations). - */ - if (p->key.dboid != MyDatabaseId && - p->key.dboid != InvalidOid && - !kind_info->accessed_across_databases) - continue; - - if (p->dropped) - continue; + dshash_seq_init(&hstat, pgStatLocal.all_hashes[h], false); + while ((p = dshash_seq_next(&hstat)) != NULL) + { + PgStat_Kind kind = p->key.kind; + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + bool found; + PgStat_SnapshotEntry *entry; + PgStatShared_Common *stats_data; + + /* + * Check if the stats object should be included in the snapshot. + * Unless the stats kind can be accessed from all databases (e.g., + * database stats themselves), we only include stats for the + * current database or objects not associated with a database + * (e.g. shared relations). + */ + if (p->key.dboid != MyDatabaseId && + p->key.dboid != InvalidOid && + !kind_info->accessed_across_databases) + continue; - Assert(pg_atomic_read_u32(&p->refcount) > 0); + if (p->dropped) + continue; - stats_data = dsa_get_address(pgStatLocal.dsa, p->body); - Assert(stats_data); + Assert(pg_atomic_read_u32(&p->refcount) > 0); - entry = pgstat_snapshot_insert(pgStatLocal.snapshot.stats, p->key, &found); - Assert(!found); + stats_data = dsa_get_address(pgStatLocal.dsa, p->body); + Assert(stats_data); - entry->data = MemoryContextAlloc(pgStatLocal.snapshot.context, - pgstat_get_entry_len(kind)); + entry = pgstat_snapshot_insert(pgStatLocal.snapshot.stats, p->key, &found); + Assert(!found); - /* - * Acquire the LWLock directly instead of using - * pg_stat_lock_entry_shared() which requires a reference. - */ - LWLockAcquire(&stats_data->lock, LW_SHARED); - memcpy(entry->data, - pgstat_get_entry_data(kind, stats_data), - pgstat_get_entry_len(kind)); - LWLockRelease(&stats_data->lock); + entry->data = MemoryContextAlloc(pgStatLocal.snapshot.context, + pgstat_get_entry_len(kind)); + + /* + * Acquire the LWLock directly instead of using + * pg_stat_lock_entry_shared() which requires a reference. + */ + LWLockAcquire(&stats_data->lock, LW_SHARED); + memcpy(entry->data, + pgstat_get_entry_data(kind, stats_data), + pgstat_get_entry_len(kind)); + LWLockRelease(&stats_data->lock); + } + dshash_seq_term(&hstat); } - dshash_seq_term(&hstat); /* * Build snapshot of all fixed-numbered stats. @@ -1540,6 +1543,10 @@ pgstat_register_kind(PgStat_Kind kind, const PgStat_KindInfo *kind_info) ereport(ERROR, (errmsg("custom cumulative statistics property is invalid"), errhint("Custom cumulative statistics cannot use entry count tracking for fixed-numbered objects."))); + if (kind_info->own_hash) + ereport(ERROR, + (errmsg("custom cumulative statistics property is invalid"), + errhint("Custom cumulative statistics cannot use a dedicated hash table for fixed-numbered objects."))); } /* @@ -1683,78 +1690,81 @@ pgstat_write_statsfile(void) /* * Walk through the stats entries */ - dshash_seq_init(&hstat, pgStatLocal.shared_hash, false); - while ((ps = dshash_seq_next(&hstat)) != NULL) + for (int h = 0; h < pgStatLocal.num_hashes; h++) { - PgStatShared_Common *shstats; - const PgStat_KindInfo *kind_info = NULL; - - CHECK_FOR_INTERRUPTS(); - - /* - * We should not see any "dropped" entries when writing the stats - * file, as all backends and auxiliary processes should have cleaned - * up their references before they terminated. - * - * However, since we are already shutting down, it is not worth - * crashing the server over any potential cleanup issues, so we simply - * skip such entries if encountered. - */ - Assert(!ps->dropped); - if (ps->dropped) - continue; - - /* - * This discards data related to custom stats kinds that are unknown - * to this process. - */ - if (!pgstat_is_kind_valid(ps->key.kind)) + dshash_seq_init(&hstat, pgStatLocal.all_hashes[h], false); + while ((ps = dshash_seq_next(&hstat)) != NULL) { - elog(WARNING, "found unknown stats entry %u/%u/%" PRIu64, - ps->key.kind, ps->key.dboid, - ps->key.objid); - continue; - } - - shstats = (PgStatShared_Common *) dsa_get_address(pgStatLocal.dsa, ps->body); + PgStatShared_Common *shstats; + const PgStat_KindInfo *kind_info = NULL; + + CHECK_FOR_INTERRUPTS(); + + /* + * We should not see any "dropped" entries when writing the stats + * file, as all backends and auxiliary processes should have + * cleaned up their references before they terminated. + * + * However, since we are already shutting down, it is not worth + * crashing the server over any potential cleanup issues, so we + * simply skip such entries if encountered. + */ + Assert(!ps->dropped); + if (ps->dropped) + continue; - kind_info = pgstat_get_kind_info(ps->key.kind); + /* + * This discards data related to custom stats kinds that are + * unknown to this process. + */ + if (!pgstat_is_kind_valid(ps->key.kind)) + { + elog(WARNING, "found unknown stats entry %u/%u/%" PRIu64, + ps->key.kind, ps->key.dboid, + ps->key.objid); + continue; + } - /* if not dropped the valid-entry refcount should exist */ - Assert(pg_atomic_read_u32(&ps->refcount) > 0); + shstats = (PgStatShared_Common *) dsa_get_address(pgStatLocal.dsa, ps->body); - /* skip if no need to write to file */ - if (!kind_info->write_to_file) - continue; + kind_info = pgstat_get_kind_info(ps->key.kind); - if (!kind_info->to_serialized_name) - { - /* normal stats entry, identified by PgStat_HashKey */ - fputc(PGSTAT_FILE_ENTRY_HASH, fpout); - pgstat_write_chunk_s(fpout, &ps->key); - } - else - { - /* stats entry identified by name on disk (e.g. slots) */ - NameData name; + /* if not dropped the valid-entry refcount should exist */ + Assert(pg_atomic_read_u32(&ps->refcount) > 0); - kind_info->to_serialized_name(&ps->key, shstats, &name); + /* skip if no need to write to file */ + if (!kind_info->write_to_file) + continue; - fputc(PGSTAT_FILE_ENTRY_NAME, fpout); - pgstat_write_chunk_s(fpout, &ps->key.kind); - pgstat_write_chunk_s(fpout, &name); + if (!kind_info->to_serialized_name) + { + /* normal stats entry, identified by PgStat_HashKey */ + fputc(PGSTAT_FILE_ENTRY_HASH, fpout); + pgstat_write_chunk_s(fpout, &ps->key); + } + else + { + /* stats entry identified by name on disk (e.g. slots) */ + NameData name; + + kind_info->to_serialized_name(&ps->key, shstats, &name); + + fputc(PGSTAT_FILE_ENTRY_NAME, fpout); + pgstat_write_chunk_s(fpout, &ps->key.kind); + pgstat_write_chunk_s(fpout, &name); + } + + /* Write except the header part of the entry */ + pgstat_write_chunk(fpout, + pgstat_get_entry_data(ps->key.kind, shstats), + pgstat_get_entry_len(ps->key.kind)); + + /* Write more data for the entry, if required */ + if (kind_info->to_serialized_data) + kind_info->to_serialized_data(&ps->key, shstats, fpout); } - - /* Write except the header part of the entry */ - pgstat_write_chunk(fpout, - pgstat_get_entry_data(ps->key.kind, shstats), - pgstat_get_entry_len(ps->key.kind)); - - /* Write more data for the entry, if required */ - if (kind_info->to_serialized_data) - kind_info->to_serialized_data(&ps->key, shstats, fpout); + dshash_seq_term(&hstat); } - dshash_seq_term(&hstat); /* * No more output to be done. Close the temp file and replace the old @@ -2021,12 +2031,12 @@ pgstat_read_statsfile(void) * putting all stats into checkpointer's * pgStatEntryRefHash would be wasted effort and memory. */ - p = dshash_find_or_insert(pgStatLocal.shared_hash, &key, &found); + p = dshash_find_or_insert(pgstat_get_hash_for_kind(key.kind), &key, &found); /* don't allow duplicate entries */ if (found) { - dshash_release_lock(pgStatLocal.shared_hash, p); + dshash_release_lock(pgstat_get_hash_for_kind(key.kind), p); elog(WARNING, "found duplicate stats entry %u/%u/%" PRIu64 " of type %c", key.kind, key.dboid, key.objid, t); @@ -2034,7 +2044,7 @@ pgstat_read_statsfile(void) } header = pgstat_init_entry(key.kind, p); - dshash_release_lock(pgStatLocal.shared_hash, p); + dshash_release_lock(pgstat_get_hash_for_kind(key.kind), p); if (header == NULL) { /* diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 5ea3f1973f9..411d7805e80 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -210,6 +210,25 @@ StatsShmemInit(void *arg) /* lift limit set above */ dsa_set_size_limit(dsa, -1); + /* + * Create dedicated hash tables for kinds that have variable-numbered + * stats and set own_hash to true. + */ + memset(ctl->kind_hash_valid, 0, sizeof(ctl->kind_hash_valid)); + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + dshash_table *kind_dsh; + + if (!kind_info || kind_info->fixed_amount || !kind_info->own_hash) + continue; + + kind_dsh = dshash_create(dsa, &dsh_params, NULL); + ctl->kind_hash_handles[kind] = dshash_get_hash_table_handle(kind_dsh); + ctl->kind_hash_valid[kind] = true; + dshash_detach(kind_dsh); + } + /* * Postmaster will never access these again, thus free the local * dsa/dshash references. @@ -257,6 +276,8 @@ pgstat_attach_shmem(void) { MemoryContext oldcontext; + pgStatLocal.num_hashes = 0; + Assert(pgStatLocal.dsa == NULL); /* stats shared memory persists for the backend lifetime */ @@ -270,6 +291,26 @@ pgstat_attach_shmem(void) pgStatLocal.shmem->hash_handle, NULL); + /* Attach dedicated hash tables */ + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + { + if (pgStatLocal.shmem->kind_hash_valid[kind]) + pgStatLocal.kind_hash[kind] = + dshash_attach(pgStatLocal.dsa, &dsh_params, + pgStatLocal.shmem->kind_hash_handles[kind], + NULL); + else + pgStatLocal.kind_hash[kind] = NULL; + } + + /* Populate array of all hash tables */ + pgStatLocal.all_hashes[pgStatLocal.num_hashes++] = pgStatLocal.shared_hash; + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + { + if (pgStatLocal.kind_hash[kind] != NULL) + pgStatLocal.all_hashes[pgStatLocal.num_hashes++] = pgStatLocal.kind_hash[kind]; + } + MemoryContextSwitchTo(oldcontext); } @@ -284,6 +325,16 @@ pgstat_detach_shmem(void) dshash_detach(pgStatLocal.shared_hash); pgStatLocal.shared_hash = NULL; + /* Detach dedicated hash tables */ + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + { + if (pgStatLocal.kind_hash[kind] != NULL) + { + dshash_detach(pgStatLocal.kind_hash[kind]); + pgStatLocal.kind_hash[kind] = NULL; + } + } + dsa_detach(pgStatLocal.dsa); /* @@ -404,7 +455,7 @@ pgstat_acquire_entry_ref(PgStat_EntryRef *entry_ref, pg_atomic_fetch_add_u32(&shhashent->refcount, 1); - dshash_release_lock(pgStatLocal.shared_hash, shhashent); + dshash_release_lock(pgstat_get_hash_for_kind(shhashent->key.kind), shhashent); entry_ref->shared_stats = shheader; entry_ref->shared_entry = shhashent; @@ -478,6 +529,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, PgStatShared_HashEntry *shhashent; PgStatShared_Common *shheader = NULL; PgStat_EntryRef *entry_ref; + dshash_table *hash; key.kind = kind; key.dboid = dboid; @@ -521,7 +573,9 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, * Do a lookup in the hash table first - it's quite likely that the entry * already exists, and that way we only need a shared lock. */ - shhashent = dshash_find(pgStatLocal.shared_hash, &key, false); + hash = pgstat_get_hash_for_kind(kind); + + shhashent = dshash_find(hash, &key, false); if (create && !shhashent) { @@ -532,7 +586,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, * lookup. If so, fall through to the same path as if we'd have if it * already had been created before the dshash_find() calls. */ - shhashent = dshash_find_or_insert(pgStatLocal.shared_hash, &key, &shfound); + shhashent = dshash_find_or_insert(hash, &key, &shfound); if (!shfound) { shheader = pgstat_init_entry(kind, shhashent); @@ -542,7 +596,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, * Failed the allocation of a new entry, so clean up the * shared hashtable before giving up. */ - dshash_delete_entry(pgStatLocal.shared_hash, shhashent); + dshash_delete_entry(hash, shhashent); ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), @@ -598,7 +652,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, } else if (shhashent->dropped) { - dshash_release_lock(pgStatLocal.shared_hash, shhashent); + dshash_release_lock(hash, shhashent); pgstat_release_entry_ref(key, entry_ref, false); return NULL; @@ -647,7 +701,7 @@ pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, /* only dropped entries can reach a 0 refcount */ Assert(entry_ref->shared_entry->dropped); - shent = dshash_find(pgStatLocal.shared_hash, + shent = dshash_find(pgstat_get_hash_for_kind(key.kind), &entry_ref->shared_entry->key, true); if (!shent) @@ -672,7 +726,7 @@ pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, * Shared stats entry has been reinitialized, so do not drop * its shared entry, only release its lock. */ - dshash_release_lock(pgStatLocal.shared_hash, shent); + dshash_release_lock(pgstat_get_hash_for_kind(key.kind), shent); } } } @@ -886,7 +940,7 @@ pgstat_free_entry(PgStatShared_HashEntry *shent, dshash_seq_status *hstat) pdsa = shent->body; if (!hstat) - dshash_delete_entry(pgStatLocal.shared_hash, shent); + dshash_delete_entry(pgstat_get_hash_for_kind(kind), shent); else dshash_delete_current(hstat); @@ -929,7 +983,7 @@ pgstat_drop_entry_internal(PgStatShared_HashEntry *shent, else { if (!hstat) - dshash_release_lock(pgStatLocal.shared_hash, shent); + dshash_release_lock(pgstat_get_hash_for_kind(shent->key.kind), shent); return false; } } @@ -959,25 +1013,29 @@ pgstat_drop_database_and_contents(Oid dboid) pgstat_release_db_entry_refs(dboid); /* some of the dshash entries are to be removed, take exclusive lock. */ - dshash_seq_init(&hstat, pgStatLocal.shared_hash, true); - while ((p = dshash_seq_next(&hstat)) != NULL) + for (int h = 0; h < pgStatLocal.num_hashes; h++) { - if (p->dropped) - continue; + dshash_seq_init(&hstat, pgStatLocal.all_hashes[h], true); + while ((p = dshash_seq_next(&hstat)) != NULL) + { + if (p->dropped) + continue; - if (p->key.dboid != dboid) - continue; + if (p->key.dboid != dboid) + continue; - if (!pgstat_drop_entry_internal(p, &hstat)) - { - /* - * Even statistics for a dropped database might currently be - * accessed (consider e.g. database stats for pg_stat_database). - */ - not_freed_count++; + if (!pgstat_drop_entry_internal(p, &hstat)) + { + /* + * Even statistics for a dropped database might currently be + * accessed (consider e.g. database stats for + * pg_stat_database). + */ + not_freed_count++; + } } + dshash_seq_term(&hstat); } - dshash_seq_term(&hstat); /* * If some of the stats data could not be freed, signal the reference @@ -1023,8 +1081,8 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, true); } - /* mark entry in shared hashtable as deleted, drop if possible */ - shent = dshash_find(pgStatLocal.shared_hash, &key, true); + /* mark entry in the kind's hashtable as deleted, drop if possible */ + shent = dshash_find(pgstat_get_hash_for_kind(kind), &key, true); if (shent) { if (shent->dropped) @@ -1037,7 +1095,7 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, shent->key.objid, pg_atomic_read_u32(&shent->refcount), pg_atomic_read_u32(&shent->generation)); - dshash_release_lock(pgStatLocal.shared_hash, shent); + dshash_release_lock(pgstat_get_hash_for_kind(kind), shent); return true; } @@ -1057,8 +1115,8 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, } /* - * Scan through the shared hashtable of stats, dropping statistics if - * approved by the optional do_drop() function. + * Scan through all stats hash tables, dropping statistics if approved by the + * optional do_drop() function. */ void pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), @@ -1069,37 +1127,40 @@ pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), uint64 not_freed_count = 0; /* entries are removed, take an exclusive lock */ - dshash_seq_init(&hstat, pgStatLocal.shared_hash, true); - while ((ps = dshash_seq_next(&hstat)) != NULL) + for (int h = 0; h < pgStatLocal.num_hashes; h++) { - if (ps->dropped) - continue; + dshash_seq_init(&hstat, pgStatLocal.all_hashes[h], true); + while ((ps = dshash_seq_next(&hstat)) != NULL) + { + if (ps->dropped) + continue; - if (do_drop != NULL && !do_drop(ps, match_data)) - continue; + if (do_drop != NULL && !do_drop(ps, match_data)) + continue; - /* delete local reference */ - if (pgStatEntryRefHash) - { - PgStat_EntryRefHashEntry *lohashent = - pgstat_entry_ref_hash_lookup(pgStatEntryRefHash, ps->key); + /* delete local reference */ + if (pgStatEntryRefHash) + { + PgStat_EntryRefHashEntry *lohashent = + pgstat_entry_ref_hash_lookup(pgStatEntryRefHash, ps->key); - if (lohashent) - pgstat_release_entry_ref(lohashent->key, lohashent->entry_ref, - true); - } + if (lohashent) + pgstat_release_entry_ref(lohashent->key, lohashent->entry_ref, + true); + } - if (!pgstat_drop_entry_internal(ps, &hstat)) - not_freed_count++; + if (!pgstat_drop_entry_internal(ps, &hstat)) + not_freed_count++; + } + dshash_seq_term(&hstat); } - dshash_seq_term(&hstat); if (not_freed_count > 0) pgstat_request_entry_refs_gc(); } /* - * Scan through the shared hashtable of stats and drop all entries. + * Scan through all stats hash tables and drop all entries. */ void pgstat_drop_all_entries(void) @@ -1140,8 +1201,8 @@ pgstat_reset_entry(PgStat_Kind kind, Oid dboid, uint64 objid, TimestampTz ts) } /* - * Scan through the shared hashtable of stats, resetting statistics if - * approved by the provided do_reset() function. + * Scan through all stats hash tables, resetting statistics if approved by the + * provided do_reset() function. */ void pgstat_reset_matching_entries(bool (*do_reset) (PgStatShared_HashEntry *, Datum), @@ -1151,26 +1212,29 @@ pgstat_reset_matching_entries(bool (*do_reset) (PgStatShared_HashEntry *, Datum) PgStatShared_HashEntry *p; /* dshash entry is not modified, take shared lock */ - dshash_seq_init(&hstat, pgStatLocal.shared_hash, false); - while ((p = dshash_seq_next(&hstat)) != NULL) + for (int h = 0; h < pgStatLocal.num_hashes; h++) { - PgStatShared_Common *header; + dshash_seq_init(&hstat, pgStatLocal.all_hashes[h], false); + while ((p = dshash_seq_next(&hstat)) != NULL) + { + PgStatShared_Common *header; - if (p->dropped) - continue; + if (p->dropped) + continue; - if (!do_reset(p, match_data)) - continue; + if (!do_reset(p, match_data)) + continue; - header = dsa_get_address(pgStatLocal.dsa, p->body); + header = dsa_get_address(pgStatLocal.dsa, p->body); - LWLockAcquire(&header->lock, LW_EXCLUSIVE); + LWLockAcquire(&header->lock, LW_EXCLUSIVE); - shared_stat_reset_contents(p->key.kind, header, ts); + shared_stat_reset_contents(p->key.kind, header, ts); - LWLockRelease(&header->lock); + LWLockRelease(&header->lock); + } + dshash_seq_term(&hstat); } - dshash_seq_term(&hstat); } static bool diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 3ca4f454895..d1c583a05f9 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -251,6 +251,12 @@ typedef struct PgStat_KindInfo */ bool track_entry_count:1; + /* + * Should entries of this kind be stored in a dedicated dshash table + * rather than the shared hash table? For variable-numbered stats only. + */ + bool own_hash:1; + /* * The size of an entry in the shared stats hash table (pointed to by * PgStatShared_HashEntry->body). For fixed-numbered statistics, this is @@ -548,6 +554,10 @@ typedef struct PgStat_ShmemControl */ dshash_table_handle hash_handle; /* shared dbstat hash */ + /* Per-kind hash handles for kinds with own_hash set */ + dshash_table_handle kind_hash_handles[PGSTAT_KIND_MAX + 1]; + bool kind_hash_valid[PGSTAT_KIND_MAX + 1]; + /* Has the stats system already been shut down? Just a debugging check. */ bool is_shutdown; @@ -638,6 +648,11 @@ typedef struct PgStat_LocalState PgStat_ShmemControl *shmem; dsa_area *dsa; dshash_table *shared_hash; + dshash_table *kind_hash[PGSTAT_KIND_MAX + 1]; + + /* All hash tables to iterate (shared + per-kind), built at attach time */ + dshash_table *all_hashes[PGSTAT_KIND_MAX + 1]; + int num_hashes; /* the current statistics snapshot */ PgStat_Snapshot snapshot; @@ -1057,4 +1072,18 @@ pgstat_get_custom_snapshot_data(PgStat_Kind kind) return pgStatLocal.snapshot.custom_data[idx]; } +/* + * Returns the dshash table for a given kind. Kinds with own_hash set have + * a dedicated hash table; others use the shared hash. + */ +static inline dshash_table * +pgstat_get_hash_for_kind(PgStat_Kind kind) +{ + if (pgStatLocal.kind_hash[kind] != NULL) + return pgStatLocal.kind_hash[kind]; + + Assert(!pgstat_get_kind_info(kind)->own_hash); + return pgStatLocal.shared_hash; +} + #endif /* PGSTAT_INTERNAL_H */ diff --git a/src/test/modules/test_custom_stats/t/001_custom_stats.pl b/src/test/modules/test_custom_stats/t/001_custom_stats.pl index 9e6a7a38577..1bd4e35b31d 100644 --- a/src/test/modules/test_custom_stats/t/001_custom_stats.pl +++ b/src/test/modules/test_custom_stats/t/001_custom_stats.pl @@ -8,6 +8,9 @@ # - Persistence across restarts. # - Loss after crash recovery. # - Resets for fixed-sized stats. +# +# Variable-sized stats are tested with both own_hash=true (dedicated +# dshash) and own_hash=false (shared dshash) to cover both paths. use strict; use warnings FATAL => 'all'; @@ -17,6 +20,7 @@ use PostgreSQL::Test::Utils; use Test::More; use File::Copy; +my $result; my $node = PostgreSQL::Test::Cluster->new('main'); $node->init; $node->append_conf('postgresql.conf', @@ -27,82 +31,100 @@ $node->start; $node->safe_psql('postgres', q(CREATE EXTENSION test_custom_var_stats)); $node->safe_psql('postgres', q(CREATE EXTENSION test_custom_fixed_stats)); -# Create entries for variable-sized stats. -$node->safe_psql('postgres', - q(select test_custom_stats_var_create('entry1', 'Test entry 1'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_create('entry2', 'Test entry 2'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_create('entry3', 'Test entry 3'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_create('entry4', 'Test entry 4'))); - -# Update counters: entry1=2, entry2=3, entry3=2, entry4=3, fixed=3 -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry1'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry1'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry2'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry2'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry2'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry3'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry3'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry4'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry4'))); -$node->safe_psql('postgres', - q(select test_custom_stats_var_update('entry4'))); +# Test variable-sized stats with both own_hash configurations (true and false). +foreach my $use_own_hash (qw(true false)) +{ + + # Restart with the appropriate setting. + $node->append_conf('postgresql.conf', + "test_custom_var_stats.use_own_hash = $use_own_hash"); + $node->restart; + + $result = $node->safe_psql('postgres', + q(select test_custom_stats_var_is_own_hash())); + is( $result, + $use_own_hash eq 'true' ? 't' : 'f', + "check if dedicated hash is allocated (own_hash=$use_own_hash)"); + + # Create entries for variable-sized stats. + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry1', 'Test entry 1'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry2', 'Test entry 2'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry3', 'Test entry 3'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry4', 'Test entry 4'))); + + # Update counters: entry1=2, entry2=3, entry3=2, entry4=3 + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry1'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry1'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry2'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry2'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry2'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry3'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry3'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry4'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry4'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_update('entry4'))); + + # Test data reports. + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry1'))); + is( $result, + "entry1|2|Test entry 1", + "report for variable-sized data of entry1"); + + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry2'))); + is( $result, + "entry2|3|Test entry 2", + "report for variable-sized data of entry2"); + + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry3'))); + is( $result, + "entry3|2|Test entry 3", + "report for variable-sized data of entry3"); + + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry4'))); + is( $result, + "entry4|3|Test entry 4", + "report for variable-sized data of entry4"); + + # Test drop of variable-sized stats. + $node->safe_psql('postgres', + q(select * from test_custom_stats_var_drop('entry3'))); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry3'))); + is($result, "", "entry3 not found after drop"); + $node->safe_psql('postgres', + q(select * from test_custom_stats_var_drop('entry4'))); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry4'))); + is($result, "", "entry4 not found after drop"); +} + +# fixed-sized stats are updated 3 times, so the report below should match. $node->safe_psql('postgres', q(select test_custom_stats_fixed_update())); $node->safe_psql('postgres', q(select test_custom_stats_fixed_update())); $node->safe_psql('postgres', q(select test_custom_stats_fixed_update())); -# Test data reports. -my $result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry1'))); -is( $result, - "entry1|2|Test entry 1", - "report for variable-sized data of entry1"); - -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry2'))); -is( $result, - "entry2|3|Test entry 2", - "report for variable-sized data of entry2"); - -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry3'))); -is( $result, - "entry3|2|Test entry 3", - "report for variable-sized data of entry3"); - -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry4'))); -is( $result, - "entry4|3|Test entry 4", - "report for variable-sized data of entry4"); - $result = $node->safe_psql('postgres', q(select * from test_custom_stats_fixed_report())); is($result, "3|", "report for fixed-sized stats"); -# Test drop of variable-sized stats. -$node->safe_psql('postgres', - q(select * from test_custom_stats_var_drop('entry3'))); -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry3'))); -is($result, "", "entry3 not found after drop"); -$node->safe_psql('postgres', - q(select * from test_custom_stats_var_drop('entry4'))); -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry4'))); -is($result, "", "entry4 not found after drop"); - # Test persistence across clean restart. $node->stop(); $node->start(); diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql b/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql index 5ed8cfc2dcf..d8c07d874f9 100644 --- a/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql +++ b/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql @@ -24,3 +24,13 @@ CREATE FUNCTION test_custom_stats_var_report(INOUT name TEXT, RETURNS SETOF record AS 'MODULE_PATHNAME', 'test_custom_stats_var_report' LANGUAGE C STRICT PARALLEL UNSAFE; + +CREATE FUNCTION test_custom_stats_var_scan() +RETURNS BIGINT +AS 'MODULE_PATHNAME', 'test_custom_stats_var_scan' +LANGUAGE C STRICT PARALLEL UNSAFE; + +CREATE FUNCTION test_custom_stats_var_is_own_hash() +RETURNS BOOLEAN +AS 'MODULE_PATHNAME', 'test_custom_stats_var_is_own_hash' +LANGUAGE C STRICT PARALLEL UNSAFE; diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/src/test/modules/test_custom_stats/test_custom_var_stats.c index 863d6a52492..ffa0a95d22d 100644 --- a/src/test/modules/test_custom_stats/test_custom_var_stats.c +++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c @@ -15,6 +15,7 @@ #include "access/htup_details.h" #include "common/hashfn.h" #include "funcapi.h" +#include "utils/guc.h" #include "storage/dsm_registry.h" #include "storage/fd.h" #include "utils/builtins.h" @@ -105,7 +106,27 @@ static void test_custom_stats_var_finish(PgStat_StatsFileOp status); *-------------------------------------------------------------------------- */ -static const PgStat_KindInfo custom_stats = { +/* Whether to use a dedicated dshash for this kind */ +static bool test_custom_stats_use_own_hash = false; + +static const PgStat_KindInfo custom_stats_own_hash = { + .name = "test_custom_var_stats", + .fixed_amount = false, /* variable number of entries */ + .write_to_file = true, /* persist across restarts */ + .track_entry_count = true, /* count active entries */ + .accessed_across_databases = true, /* global statistics */ + .own_hash = true, /* use dedicated dshash */ + .shared_size = sizeof(PgStatShared_CustomVarEntry), + .shared_data_off = offsetof(PgStatShared_CustomVarEntry, stats), + .shared_data_len = sizeof(((PgStatShared_CustomVarEntry *) 0)->stats), + .pending_size = sizeof(PgStat_StatCustomVarEntry), + .flush_pending_cb = test_custom_stats_var_flush_pending_cb, + .to_serialized_data = test_custom_stats_var_to_serialized_data, + .from_serialized_data = test_custom_stats_var_from_serialized_data, + .finish = test_custom_stats_var_finish, +}; + +static const PgStat_KindInfo custom_stats_shared_hash = { .name = "test_custom_var_stats", .fixed_amount = false, /* variable number of entries */ .write_to_file = true, /* persist across restarts */ @@ -133,8 +154,25 @@ _PG_init(void) if (!process_shared_preload_libraries_in_progress) return; - /* Register custom statistics kind */ - pgstat_register_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, &custom_stats); + /* + * test_custom_stats_use_own_hash = (on|off) + * + * Use the shared hash (default) or a dedicated hash for the + * test_custom_var_stats kind. + */ + DefineCustomBoolVariable("test_custom_var_stats.use_own_hash", + "Use dedicated dshash for test custom var stats", + NULL, + &test_custom_stats_use_own_hash, + false, + PGC_POSTMASTER, + 0, + NULL, NULL, NULL); + + /* Register with the appropriate kind info */ + pgstat_register_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, + test_custom_stats_use_own_hash ? + &custom_stats_own_hash : &custom_stats_shared_hash); } /*-------------------------------------------------------------------------- @@ -692,3 +730,62 @@ test_custom_stats_var_report(PG_FUNCTION_ARGS) SRF_RETURN_DONE(funcctx); } + +/* + * test_custom_stats_var_scan + * Count entries by scanning the kind's dedicated dshash directly + * + * Exercises pgstat_get_hash_for_kind() and dshash_seq iteration on + * a dedicated hash table. + */ +PG_FUNCTION_INFO_V1(test_custom_stats_var_scan); +Datum +test_custom_stats_var_scan(PG_FUNCTION_ARGS) +{ + dshash_table *hash; + dshash_seq_status hstat; + PgStatShared_HashEntry *p; + int64 count = 0; + + hash = pgstat_get_hash_for_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS); + + dshash_seq_init(&hstat, hash, false); + while ((p = dshash_seq_next(&hstat)) != NULL) + { + if (p->dropped) + continue; + + if (p->key.kind != PGSTAT_KIND_TEST_CUSTOM_VAR_STATS) + continue; + + count++; + } + dshash_seq_term(&hstat); + + PG_RETURN_INT64(count); +} + +/* + * test_custom_stats_var_is_own_hash + * Verify whether the kind uses a dedicated dshash + * + * Scans pgStatLocal.all_hashes[] looking for the hash returned by + * pgstat_get_hash_for_kind(). Index 0 is always the shared hash, + * so finding it at a non-zero index confirms it has its own hash. + */ +PG_FUNCTION_INFO_V1(test_custom_stats_var_is_own_hash); +Datum +test_custom_stats_var_is_own_hash(PG_FUNCTION_ARGS) +{ + dshash_table *hash; + + hash = pgstat_get_hash_for_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS); + + for (int i = 0; i < pgStatLocal.num_hashes; i++) + { + if (pgStatLocal.all_hashes[i] == hash) + PG_RETURN_BOOL(i != 0); + } + + PG_RETURN_BOOL(false); +} -- 2.50.1 (Apple Git-155)