From c0c7ac1a429e4b2958f85d70d196f69dc743de79 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Thu, 11 Jun 2026 07:30:38 -0500 Subject: [PATCH v3 1/1] pgstat: allow a stats kind to use its own dedicated dshash Variable-numbered pgstats entries currently all live in one shared dshash table. This works for small stats kinds, but a future conversion of pg_stat_statements to a custom cumulative stats kind would create a large number of entries and would frequently need to scan all of them. Add an own_hash option for variable-numbered stats kinds, allowing such a kind to use a dedicated dshash table backed by its own DSA area. This provides the infrastructure needed for that kind of extension without making scans of large stats kinds pass through unrelated pgstats entries, and without putting all such activity on the shared hash partitions. At attach time, each registered variable-numbered stats kind is mapped to either the shared hash or its dedicated hash. Lookups and inserts use that mapping via pgstat_get_hash_for_kind(), while full-table operations iterate the attached set of stats hash tables. New entries are allocated before insertion into the selected hash table and inserted with dshash_find_or_insert_extended(..., DSHASH_INSERT_NO_OOM), preserving the OOM-safe behavior of the shared pgstats hash path. 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 test both the shared and dedicated hash paths. Expose the own_hash mode in pg_stat_kind_info. Reviewed-by: Michael Paquier Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/anX2J5yYsO9ae2Tq@bdtpg --- doc/src/sgml/monitoring.sgml | 13 + src/backend/catalog/system_views.sql | 1 + src/backend/utils/activity/pgstat.c | 249 ++++++------ src/backend/utils/activity/pgstat_kind.c | 7 +- src/backend/utils/activity/pgstat_shmem.c | 369 ++++++++++++------ src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 6 +- src/include/utils/pgstat_internal.h | 73 ++-- .../test_custom_stats/t/001_custom_stats.pl | 250 +++++++----- .../test_custom_var_stats--1.0.sql | 10 + .../test_custom_stats/test_custom_var_stats.c | 82 +++- src/test/regress/expected/rules.out | 3 +- 12 files changed, 704 insertions(+), 361 deletions(-) diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 6337d2a3d25..67ae6fd5f7e 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -3574,6 +3574,19 @@ description | Waiting for a newly initialized WAL file to reach durable storage + + + + own_hash boolean + + + For variable-numbered statistics kinds, true if entries of this kind + are stored in a dedicated hash table, false if they are stored in the + shared hash table. False for fixed-numbered statistics kinds. + + + + diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 809b9c0f1e4..ddf801307eb 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1290,6 +1290,7 @@ CREATE VIEW pg_stat_kind_info AS k.fixed_amount, k.accessed_across_databases, k.write_to_file, + k.own_hash, k.entry_count FROM pg_stat_get_kind_info() k; diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 0b44a53ebe9..ef0615e974d 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -18,7 +18,7 @@ * Fixed-numbered stats are stored in plain (non-dynamic) shared memory. * * Statistics for variable-numbered objects are stored in dynamic shared - * memory and can be found via a dshash hashtable. The statistics counters are + * memory and can be found via dshash hashtables. The statistics counters are * not part of the dshash entry (PgStatShared_HashEntry) directly, but are * separately allocated (PgStatShared_HashEntry->body). The separate * allocation allows different kinds of statistics to be stored in the same @@ -29,12 +29,12 @@ * that way at runtime. A wider identifier can be used when serializing to * disk (used for replication slot stats). * - * To avoid contention on the shared hashtable, each backend has a - * backend-local hashtable (pgStatEntryRefHash) in front of the shared - * hashtable, containing references (PgStat_EntryRef) to shared hashtable - * entries. The shared hashtable only needs to be accessed when no prior + * To avoid contention on the stats hashtables, each backend has a + * backend-local hashtable (pgStatEntryRefHash) in front of the stats + * hashtables, containing references (PgStat_EntryRef) to stats hashtable + * entries. The stats hashtables only need to be accessed when no prior * reference is found in the local hashtable. Besides pointing to the - * shared hashtable entry (PgStatShared_HashEntry) PgStat_EntryRef also + * stats hashtable entry (PgStatShared_HashEntry) PgStat_EntryRef also * contains a pointer to the shared statistics data, as a process-local * address, to reduce access costs. * @@ -1199,52 +1199,58 @@ 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; + dshash_table *hash = pgStatLocal.all_hashes[h]; + dsa_area *dsa = dshash_get_dsa_area(hash); - if (p->dropped) - continue; + dshash_seq_init(&hstat, hash, 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(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. @@ -1584,6 +1590,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 entry count tracking for fixed-numbered objects."))); + if (kind_info->own_hash) + ereport(ERROR, + (errmsg("failed to register custom cumulative statistics \"%s\" with ID %u", kind_info->name, kind), + errhint("Custom cumulative statistics cannot use a dedicated hash table for fixed-numbered objects."))); } else { @@ -1739,80 +1749,86 @@ 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 && status == STATS_WRITE; h++) { - PgStatShared_Common *shstats; - const PgStat_KindInfo *kind_info = NULL; + dshash_table *hash = pgStatLocal.all_hashes[h]; + dsa_area *dsa = dshash_get_dsa_area(hash); - /* - * 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, hash, 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); - - kind_info = pgstat_get_kind_info(ps->key.kind); - - /* if not dropped the valid-entry refcount should exist */ - Assert(pg_atomic_read_u32(&ps->refcount) > 0); + PgStatShared_Common *shstats; + const PgStat_KindInfo *kind_info = NULL; + + /* + * 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; - /* skip if no need to write to file */ - if (!kind_info->write_to_file) - continue; + /* + * 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 (!kind_info->to_serialized_name) - { - /* normal stats entry, identified by PgStat_HashKey */ - fputc(PGSTAT_FILE_ENTRY_HASH, fpout); - write_chunk_s(fpout, &ps->key); - } - else - { - /* stats entry identified by name on disk (e.g. slots) */ - NameData name; + shstats = (PgStatShared_Common *) dsa_get_address(dsa, ps->body); - kind_info->to_serialized_name(&ps->key, shstats, &name); + kind_info = pgstat_get_kind_info(ps->key.kind); - fputc(PGSTAT_FILE_ENTRY_NAME, fpout); - write_chunk_s(fpout, &ps->key.kind); - write_chunk_s(fpout, &name); - } + /* if not dropped the valid-entry refcount should exist */ + Assert(pg_atomic_read_u32(&ps->refcount) > 0); - /* Write except the header part of the entry */ - write_chunk(fpout, - pgstat_get_entry_data(ps->key.kind, shstats), - pgstat_get_entry_len(ps->key.kind)); + /* skip if no need to write to file */ + if (!kind_info->write_to_file) + continue; - /* Write more data for the entry, if required */ - if (kind_info->to_serialized_data && - !kind_info->to_serialized_data(&ps->key, shstats, fpout)) - { - status = STATS_DISCARD; - break; + if (!kind_info->to_serialized_name) + { + /* normal stats entry, identified by PgStat_HashKey */ + fputc(PGSTAT_FILE_ENTRY_HASH, fpout); + 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); + write_chunk_s(fpout, &ps->key.kind); + write_chunk_s(fpout, &name); + } + + /* Write except the header part of the entry */ + 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)) + { + status = STATS_DISCARD; + break; + } } + dshash_seq_term(&hstat); } - dshash_seq_term(&hstat); /* * No more output to be done. Close the temp file and replace the old @@ -1889,6 +1905,8 @@ pgstat_read_statsfile(void) PgStat_StatsFileOp status = STATS_READ; const char *statfile = PGSTAT_STAT_PERMANENT_FILENAME; PgStat_ShmemControl *shmem = pgStatLocal.shmem; + dshash_table *hash; + dsa_area *dsa; /* shouldn't be called from postmaster */ Assert(IsUnderPostmaster || !IsPostmasterEnvironment); @@ -2090,6 +2108,9 @@ pgstat_read_statsfile(void) Assert(key.kind == kind); } + hash = pgstat_get_hash_for_kind(key.kind); + dsa = dshash_get_dsa_area(hash); + /* * This intentionally doesn't use pgstat_get_entry_ref() - * putting all stats into checkpointer's @@ -2097,7 +2118,7 @@ pgstat_read_statsfile(void) * * Allocate the DSA body before inserting the hash entry. */ - chunk = pgstat_alloc_entry_body(key.kind); + chunk = pgstat_alloc_entry_body(key.kind, dsa); if (chunk == InvalidDsaPointer) { /* @@ -2110,12 +2131,12 @@ pgstat_read_statsfile(void) key.objid, t); } - p = dshash_find_or_insert_extended(pgStatLocal.shared_hash, + p = dshash_find_or_insert_extended(hash, &key, &found, DSHASH_INSERT_NO_OOM); if (!p) { - dsa_free(pgStatLocal.dsa, chunk); + dsa_free(dsa, chunk); /* * for the same reason as previously, ERROR not @@ -2129,16 +2150,16 @@ pgstat_read_statsfile(void) /* don't allow duplicate entries */ if (found) { - dshash_release_lock(pgStatLocal.shared_hash, p); - dsa_free(pgStatLocal.dsa, chunk); + dshash_release_lock(hash, p); + dsa_free(dsa, chunk); elog(WARNING, "found duplicate stats entry %u/%u/%" PRIu64 " of type %c", key.kind, key.dboid, key.objid, t); goto error; } - header = pgstat_init_entry(key.kind, p, chunk); - dshash_release_lock(pgStatLocal.shared_hash, p); + header = pgstat_init_entry(key.kind, dsa, p, chunk); + dshash_release_lock(hash, p); if (!read_chunk(fpin, pgstat_get_entry_data(key.kind, header), diff --git a/src/backend/utils/activity/pgstat_kind.c b/src/backend/utils/activity/pgstat_kind.c index 6c53b7e49bf..d0319825fe1 100644 --- a/src/backend/utils/activity/pgstat_kind.c +++ b/src/backend/utils/activity/pgstat_kind.c @@ -31,7 +31,7 @@ Datum pg_stat_get_kind_info(PG_FUNCTION_ARGS) { -#define PG_STAT_KIND_INFO_COLS 7 +#define PG_STAT_KIND_INFO_COLS 8 ReturnSetInfo *rsinfo; InitMaterializedSRF(fcinfo, 0); @@ -54,15 +54,16 @@ pg_stat_get_kind_info(PG_FUNCTION_ARGS) values[3] = BoolGetDatum(info->fixed_amount); values[4] = BoolGetDatum(info->accessed_across_databases); values[5] = BoolGetDatum(info->write_to_file); + values[6] = BoolGetDatum(info->own_hash); /* * When track_entry_count is disabled, use NULL. Fixed-sized stats * kinds report NULL here. */ if (info->track_entry_count) - values[6] = Int64GetDatum(pgstat_get_entry_count(kind)); + values[7] = Int64GetDatum(pgstat_get_entry_count(kind)); else - nulls[6] = true; + nulls[7] = true; tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls); } diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 1858d68f918..c739555574f 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -66,7 +66,7 @@ const ShmemCallbacks StatsShmemCallbacks = { .init_fn = StatsShmemInit, }; -/* parameter for the shared hash */ +/* Parameters for stats hashtables. */ static const dshash_parameters dsh_params = { sizeof(PgStat_HashKey), sizeof(PgStatShared_HashEntry), @@ -105,8 +105,8 @@ static MemoryContext pgStatEntryRefHashContext = NULL; */ /* - * The size of the shared memory allocation for stats stored in the shared - * stats hash table. This allocation will be done as part of the main shared + * The size of the shared memory allocation for stats stored in a stats + * hashtable. This allocation will be done as part of the main shared * memory, rather than dynamic shared memory, allowing it to be initialized in * postmaster. */ @@ -139,6 +139,17 @@ StatsShmemSize(void) sz = MAXALIGN(sizeof(PgStat_ShmemControl)); sz = add_size(sz, pgstat_dsa_init_size()); + /* Add per-kind DSA space for variable-numbered own_hash kinds. */ + 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 || kind_info->fixed_amount || !kind_info->own_hash) + continue; + + sz = add_size(sz, pgstat_dsa_init_size()); + } + /* Add shared memory for all the custom fixed-numbered statistics */ for (PgStat_Kind kind = PGSTAT_KIND_CUSTOM_MIN; kind <= PGSTAT_KIND_CUSTOM_MAX; kind++) { @@ -210,6 +221,41 @@ StatsShmemInit(void *arg) /* lift limit set above */ dsa_set_size_limit(dsa, -1); + /* + * Create dedicated DSA areas and 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)); + memset(ctl->raw_kind_dsa_area, 0, sizeof(ctl->raw_kind_dsa_area)); + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + dsa_area *kind_dsa; + dshash_table *kind_dsh; + + if (!kind_info || kind_info->fixed_amount || !kind_info->own_hash) + continue; + + /* Create a per-kind DSA in pre-allocated shared memory */ + ctl->raw_kind_dsa_area[kind] = p; + p += pgstat_dsa_init_size(); + kind_dsa = dsa_create_in_place(ctl->raw_kind_dsa_area[kind], + pgstat_dsa_init_size(), + LWTRANCHE_PGSTATS_DSA, NULL); + dsa_pin(kind_dsa); + + /* Temporarily limit to keep dshash in the in-place area */ + dsa_set_size_limit(kind_dsa, pgstat_dsa_init_size()); + + kind_dsh = dshash_create(kind_dsa, &dsh_params, NULL); + ctl->kind_hash_handles[kind] = dshash_get_hash_table_handle(kind_dsh); + ctl->kind_hash_valid[kind] = true; + + dsa_set_size_limit(kind_dsa, -1); + dshash_detach(kind_dsh); + dsa_detach(kind_dsa); + } + /* * Postmaster will never access these again, thus free the local * dsa/dshash references. @@ -255,20 +301,53 @@ StatsShmemInit(void *arg) void pgstat_attach_shmem(void) { + dsa_area *shared_dsa; + dshash_table *shared_hash; MemoryContext oldcontext; - Assert(pgStatLocal.dsa == NULL); + Assert(pgStatLocal.num_hashes == 0); + + pgStatLocal.num_hashes = 0; /* stats shared memory persists for the backend lifetime */ oldcontext = MemoryContextSwitchTo(TopMemoryContext); - pgStatLocal.dsa = dsa_attach_in_place(pgStatLocal.shmem->raw_dsa_area, - NULL); - dsa_pin_mapping(pgStatLocal.dsa); + shared_dsa = dsa_attach_in_place(pgStatLocal.shmem->raw_dsa_area, + NULL); + dsa_pin_mapping(shared_dsa); + + shared_hash = dshash_attach(shared_dsa, &dsh_params, + pgStatLocal.shmem->hash_handle, + NULL); - pgStatLocal.shared_hash = dshash_attach(pgStatLocal.dsa, &dsh_params, - pgStatLocal.shmem->hash_handle, - NULL); + /* Attach per-kind DSAs and hash tables. */ + pgStatLocal.all_hashes[pgStatLocal.num_hashes++] = shared_hash; + + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); + + if (pgStatLocal.shmem->kind_hash_valid[kind]) + { + dsa_area *kind_dsa; + + kind_dsa = + dsa_attach_in_place(pgStatLocal.shmem->raw_kind_dsa_area[kind], + NULL); + dsa_pin_mapping(kind_dsa); + + pgStatLocal.kind_hash[kind] = + dshash_attach(kind_dsa, &dsh_params, + pgStatLocal.shmem->kind_hash_handles[kind], + NULL); + pgStatLocal.all_hashes[pgStatLocal.num_hashes++] = + pgStatLocal.kind_hash[kind]; + } + else if (kind_info && !kind_info->fixed_amount) + pgStatLocal.kind_hash[kind] = shared_hash; + else + pgStatLocal.kind_hash[kind] = NULL; + } MemoryContextSwitchTo(oldcontext); } @@ -276,15 +355,25 @@ pgstat_attach_shmem(void) void pgstat_detach_shmem(void) { - Assert(pgStatLocal.dsa); + Assert(pgStatLocal.num_hashes > 0); /* we shouldn't leave references to shared stats */ pgstat_release_all_entry_refs(false); - dshash_detach(pgStatLocal.shared_hash); - pgStatLocal.shared_hash = NULL; + for (int h = 0; h < pgStatLocal.num_hashes; h++) + { + dsa_area *dsa = dshash_get_dsa_area(pgStatLocal.all_hashes[h]); + + dshash_detach(pgStatLocal.all_hashes[h]); + dsa_detach(dsa); + pgStatLocal.all_hashes[h] = NULL; + } + + pgStatLocal.num_hashes = 0; - dsa_detach(pgStatLocal.dsa); + /* Reset per-kind mappings. */ + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + pgStatLocal.kind_hash[kind] = NULL; /* * dsa_detach() does not decrement the DSA reference count as no segment @@ -293,7 +382,11 @@ pgstat_detach_shmem(void) */ dsa_release_in_place(pgStatLocal.shmem->raw_dsa_area); - pgStatLocal.dsa = NULL; + for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++) + { + if (pgStatLocal.shmem->raw_kind_dsa_area[kind] != NULL) + dsa_release_in_place(pgStatLocal.shmem->raw_kind_dsa_area[kind]); + } } @@ -308,11 +401,11 @@ pgstat_detach_shmem(void) * Returns InvalidDsaPointer in the event of an allocation failure. */ dsa_pointer -pgstat_alloc_entry_body(PgStat_Kind kind) +pgstat_alloc_entry_body(PgStat_Kind kind, dsa_area *dsa) { const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); - return dsa_allocate_extended(pgStatLocal.dsa, + return dsa_allocate_extended(dsa, kind_info->shared_size, DSA_ALLOC_ZERO | DSA_ALLOC_NO_OOM); } @@ -325,6 +418,7 @@ pgstat_alloc_entry_body(PgStat_Kind kind) */ PgStatShared_Common * pgstat_init_entry(PgStat_Kind kind, + dsa_area *dsa, PgStatShared_HashEntry *shhashent, dsa_pointer chunk) { @@ -347,7 +441,7 @@ pgstat_init_entry(PgStat_Kind kind, pg_atomic_init_u32(&shhashent->generation, 0); shhashent->dropped = false; - shheader = dsa_get_address(pgStatLocal.dsa, chunk); + shheader = dsa_get_address(dsa, chunk); shheader->magic = 0xdeadbeef; /* Link the new entry from the hash entry. */ @@ -363,11 +457,12 @@ pgstat_init_entry(PgStat_Kind kind, } static PgStatShared_Common * -pgstat_reinit_entry(PgStat_Kind kind, PgStatShared_HashEntry *shhashent) +pgstat_reinit_entry(PgStat_Kind kind, dsa_area *dsa, + PgStatShared_HashEntry *shhashent) { PgStatShared_Common *shheader; - shheader = dsa_get_address(pgStatLocal.dsa, shhashent->body); + shheader = dsa_get_address(dsa, shhashent->body); /* mark as not dropped anymore */ pg_atomic_fetch_add_u32(&shhashent->refcount, 1); @@ -405,6 +500,7 @@ pgstat_setup_shared_refs(void) */ static void pgstat_acquire_entry_ref(PgStat_EntryRef *entry_ref, + dshash_table *hash, PgStatShared_HashEntry *shhashent, PgStatShared_Common *shheader) { @@ -422,7 +518,7 @@ pgstat_acquire_entry_ref(PgStat_EntryRef *entry_ref, * LWLock can process a pending interrupt, and callers may catch the * resulting error and continue using the backend-local cache. */ - dshash_release_lock(pgStatLocal.shared_hash, shhashent); + dshash_release_lock(hash, shhashent); } /* @@ -506,6 +602,8 @@ 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; + dsa_area *dsa; key.kind = kind; key.dboid = dboid; @@ -517,7 +615,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, */ Assert(create || created_entry == NULL); pgstat_assert_is_up(); - Assert(pgStatLocal.shared_hash != NULL); + Assert(pgStatLocal.num_hashes > 0); Assert(!pgStatLocal.shmem->is_shutdown); pgstat_setup_memcxt(); @@ -549,7 +647,10 @@ 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); + dsa = dshash_get_dsa_area(hash); + + shhashent = dshash_find(hash, &key, false); if (create && !shhashent) { @@ -557,7 +658,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, dsa_pointer chunk; /* Allocate the stats body before inserting a hash entry. */ - chunk = pgstat_alloc_entry_body(kind); + chunk = pgstat_alloc_entry_body(kind, dsa); if (chunk == InvalidDsaPointer) { pgstat_release_entry_ref(key, entry_ref, false); @@ -573,16 +674,16 @@ 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_extended(pgStatLocal.shared_hash, + shhashent = dshash_find_or_insert_extended(hash, &key, &shfound, DSHASH_INSERT_NO_OOM); if (!shhashent) { - dsa_free(pgStatLocal.dsa, chunk); + dsa_free(dsa, chunk); /* - * Clean up the local reference when failing insert into the - * shared hashtable. + * Clean up the local reference when failing to insert into the + * stats hashtable. */ pgstat_release_entry_ref(key, entry_ref, false); ereport(ERROR, @@ -594,8 +695,8 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, if (!shfound) { - shheader = pgstat_init_entry(kind, shhashent, chunk); - pgstat_acquire_entry_ref(entry_ref, shhashent, shheader); + shheader = pgstat_init_entry(kind, dsa, shhashent, chunk); + pgstat_acquire_entry_ref(entry_ref, hash, shhashent, shheader); if (created_entry != NULL) *created_entry = true; @@ -604,7 +705,7 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, } /* Concurrent insert won; drop the unused body. */ - dsa_free(pgStatLocal.dsa, chunk); + dsa_free(dsa, chunk); } if (!shhashent) @@ -636,8 +737,8 @@ pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 objid, bool create, * stats to their plain state, while incrementing its "generation" * in the shared entry for any remaining local references. */ - shheader = pgstat_reinit_entry(kind, shhashent); - pgstat_acquire_entry_ref(entry_ref, shhashent, shheader); + shheader = pgstat_reinit_entry(kind, dsa, shhashent); + pgstat_acquire_entry_ref(entry_ref, hash, shhashent, shheader); if (created_entry != NULL) *created_entry = true; @@ -646,15 +747,15 @@ 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; } else { - shheader = dsa_get_address(pgStatLocal.dsa, shhashent->body); - pgstat_acquire_entry_ref(entry_ref, shhashent, shheader); + shheader = dsa_get_address(dsa, shhashent->body); + pgstat_acquire_entry_ref(entry_ref, hash, shhashent, shheader); return entry_ref; } @@ -685,6 +786,7 @@ pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, */ if (pg_atomic_fetch_sub_u32(&entry_ref->shared_entry->refcount, 1) == 1) { + dshash_table *hash = pgstat_get_hash_for_kind(key.kind); PgStatShared_HashEntry *shent; /* @@ -695,9 +797,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, - &entry_ref->shared_entry->key, - true); + shent = dshash_find(hash, &entry_ref->shared_entry->key, true); if (!shent) elog(ERROR, "could not find just referenced shared stats entry"); @@ -720,7 +820,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(hash, shent); } } } @@ -933,24 +1033,29 @@ pgstat_release_db_entry_refs(Oid dboid) static void pgstat_free_entry(PgStatShared_HashEntry *shent, dshash_seq_status *hstat) { + dshash_table *hash; + dsa_area *dsa; dsa_pointer pdsa; PgStat_Kind kind = shent->key.kind; + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind); /* * Fetch dsa pointer before deleting entry - that way we can free the * memory after releasing the lock. */ + hash = pgstat_get_hash_for_kind(kind); + dsa = dshash_get_dsa_area(hash); pdsa = shent->body; if (!hstat) - dshash_delete_entry(pgStatLocal.shared_hash, shent); + dshash_delete_entry(hash, shent); else dshash_delete_current(hstat); - dsa_free(pgStatLocal.dsa, pdsa); + dsa_free(dsa, pdsa); /* Decrement entry count, if required. */ - if (pgstat_get_kind_info(kind)->track_entry_count) + if (kind_info && kind_info->track_entry_count) pg_atomic_sub_fetch_u64(&pgStatLocal.shmem->entry_counts[kind - 1], 1); } @@ -986,7 +1091,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; } } @@ -1003,7 +1108,7 @@ pgstat_drop_database_and_contents(Oid dboid) Assert(OidIsValid(dboid)); - Assert(pgStatLocal.shared_hash != NULL); + Assert(pgStatLocal.num_hashes > 0); /* * This backend might very well be the only backend holding a reference to @@ -1011,30 +1116,34 @@ pgstat_drop_database_and_contents(Oid dboid) * being cleaned up till later. * * Doing this separately from the dshash iteration below avoids having to - * do so while holding a partition lock on the shared hashtable. + * do so while holding a partition lock on a stats hashtable. */ 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; + /* some of the dshash entries are to be removed, take exclusive lock. */ + 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 @@ -1062,9 +1171,19 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool missing_ok) { PgStat_HashKey key = {0}; + dshash_table *hash; PgStatShared_HashEntry *shent; bool freed = true; + if (kind < PGSTAT_KIND_MIN || kind > PGSTAT_KIND_MAX || + pgStatLocal.kind_hash[kind] == NULL) + { + Assert(missing_ok); + return true; + } + + hash = pgstat_get_hash_for_kind(kind); + key.kind = kind; key.dboid = dboid; key.objid = objid; @@ -1080,21 +1199,23 @@ 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 stats hashtable as deleted, drop if possible */ + shent = dshash_find(hash, &key, true); if (shent) { if (shent->dropped) { + const PgStat_KindInfo *kind_info = pgstat_get_kind_info(shent->key.kind); + if (!missing_ok) elog(ERROR, "trying to drop stats entry already dropped: kind=%s dboid=%u objid=%" PRIu64 " refcount=%u generation=%u", - pgstat_get_kind_info(shent->key.kind)->name, + kind_info ? kind_info->name : "unknown", shent->key.dboid, 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(hash, shent); return true; } @@ -1114,8 +1235,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 the stats hashtables, dropping statistics if approved by the + * optional do_drop() function. */ void pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), @@ -1125,38 +1246,41 @@ pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), PgStatShared_HashEntry *ps; 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; + /* entries are removed, take an exclusive lock */ + 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 the stats hashtables and drop all entries. */ void pgstat_drop_all_entries(void) @@ -1177,6 +1301,39 @@ shared_stat_reset_contents(PgStat_Kind kind, PgStatShared_Common *header, kind_info->reset_timestamp_cb(header, ts); } +static void +pgstat_reset_matching_entries_in_hash(dshash_table *hash, + bool (*do_reset) (PgStatShared_HashEntry *, Datum), + Datum match_data, + TimestampTz ts) +{ + dshash_seq_status hstat; + dsa_area *dsa = dshash_get_dsa_area(hash); + PgStatShared_HashEntry *p; + + /* dshash entry is not modified, take shared lock */ + dshash_seq_init(&hstat, hash, false); + while ((p = dshash_seq_next(&hstat)) != NULL) + { + PgStatShared_Common *header; + + if (p->dropped) + continue; + + if (!do_reset(p, match_data)) + continue; + + header = dsa_get_address(dsa, p->body); + + LWLockAcquire(&header->lock, LW_EXCLUSIVE); + + shared_stat_reset_contents(p->key.kind, header, ts); + + LWLockRelease(&header->lock); + } + dshash_seq_term(&hstat); +} + /* * Reset one variable-numbered stats entry. */ @@ -1197,37 +1354,18 @@ 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 the stats hashtables, resetting statistics if approved by the + * provided do_reset() function. */ void pgstat_reset_matching_entries(bool (*do_reset) (PgStatShared_HashEntry *, Datum), Datum match_data, TimestampTz ts) { - dshash_seq_status hstat; - 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) - { - PgStatShared_Common *header; - - if (p->dropped) - continue; - - if (!do_reset(p, match_data)) - continue; - - header = dsa_get_address(pgStatLocal.dsa, p->body); - - LWLockAcquire(&header->lock, LW_EXCLUSIVE); - - shared_stat_reset_contents(p->key.kind, header, ts); - - LWLockRelease(&header->lock); - } - dshash_seq_term(&hstat); + for (int h = 0; h < pgStatLocal.num_hashes; h++) + pgstat_reset_matching_entries_in_hash(pgStatLocal.all_hashes[h], + do_reset, + match_data, + ts); } static bool @@ -1239,7 +1377,10 @@ match_kind(PgStatShared_HashEntry *p, Datum match_data) void pgstat_reset_entries_of_kind(PgStat_Kind kind, TimestampTz ts) { - pgstat_reset_matching_entries(match_kind, Int32GetDatum(kind), ts); + pgstat_reset_matching_entries_in_hash(pgstat_get_hash_for_kind(kind), + match_kind, + Int32GetDatum(kind), + ts); } static void diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 7f7667f6179..9b6ef64a04e 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202610021 +#define CATALOG_VERSION_NO 202610061 #endif diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index f46427258e3..a9bb4fc5ebe 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -6075,9 +6075,9 @@ { oid => '8683', descr => 'statistics: information about statistics kinds', proname => 'pg_stat_get_kind_info', prorows => '20', proretset => 't', provolatile => 'v', proparallel => 'r', prorettype => 'record', - proargtypes => '', proallargtypes => '{int4,text,bool,bool,bool,bool,int8}', - proargmodes => '{o,o,o,o,o,o,o}', - proargnames => '{id,name,builtin,fixed_amount,accessed_across_databases,write_to_file,entry_count}', + proargtypes => '', proallargtypes => '{int4,text,bool,bool,bool,bool,bool,int8}', + proargmodes => '{o,o,o,o,o,o,o,o}', + proargnames => '{id,name,builtin,fixed_amount,accessed_across_databases,write_to_file,own_hash,entry_count}', prosrc => 'pg_stat_get_kind_info' }, { oid => '1136', descr => 'statistics: information about WAL activity', diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 201e57279b1..edec9934c38 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -25,10 +25,10 @@ /* * Types related to shared memory storage of statistics. * - * Per-object statistics are stored in the "shared stats" hashtable. That - * table's entries (PgStatShared_HashEntry) contain a pointer to the actual stats - * data for the object (the size of the stats data varies depending on the - * kind of stats). The table is keyed by PgStat_HashKey. + * Per-object statistics are stored in stats hashtables. Those tables' + * entries (PgStatShared_HashEntry) contain a pointer to the actual stats data + * for the object (the size of the stats data varies depending on the kind of + * stats). The tables are keyed by PgStat_HashKey. * * Once a backend has a reference to a shared stats entry, it increments the * entry's refcount. Even after stats data is dropped (e.g., due to a DROP @@ -37,14 +37,14 @@ * * These refcounts, in combination with a backend local hashtable * (pgStatEntryRefHash, with entries pointing to PgStat_EntryRef) in front of - * the shared hash table, mean that most stats work can happen without - * touching the shared hash table, reducing contention. + * the stats hashtables, mean that most stats work can happen without touching + * the stats hashtables, reducing contention. * * Once there are pending stats updates for a table PgStat_EntryRef->pending * is allocated to contain a working space for as-of-yet-unapplied stats * updates. Once the stats are flushed, PgStat_EntryRef->pending is freed. * - * Each stat kind in the shared hash table has a fixed member + * Each variable-numbered stat kind has a fixed member * PgStatShared_Common as the first element. */ @@ -186,7 +186,7 @@ typedef struct PgStat_EntryRef * 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 entries - * into the shared stats hash table. + * into stats hashtable entries. */ void *pending; dlist_node pending_node; /* membership in pgStatPending list */ @@ -252,9 +252,15 @@ typedef struct PgStat_KindInfo bool track_entry_count:1; /* - * The size of an entry in the shared stats hash table (pointed to by - * PgStatShared_HashEntry->body). For fixed-numbered statistics, this is - * the size of an entry in PgStat_ShmemControl->custom_data. + * 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 pointed to by PgStatShared_HashEntry->body. For + * fixed-numbered statistics, this is the size of an entry in + * PgStat_ShmemControl->custom_data. */ uint32 shared_size; @@ -541,7 +547,7 @@ typedef struct PgStatShared_Backend /* * Central shared memory entry for the cumulative stats system. * - * Fixed amount stats, the dynamic shared memory hash table for + * Fixed amount stats, the dynamic shared memory hashtables for * non-fixed-amount stats, as well as remaining bits and pieces are all * reached from here. */ @@ -549,12 +555,14 @@ typedef struct PgStat_ShmemControl { void *raw_dsa_area; - /* - * Stats for variable-numbered objects are kept in this shared hash table. - * See comment above PgStat_Kind for details. - */ + /* Shared hash for variable-numbered stats kinds without own_hash set. */ dshash_table_handle hash_handle; /* shared dbstat hash */ + /* Per-kind hash and DSA for kinds with own_hash set */ + dshash_table_handle kind_hash_handles[PGSTAT_KIND_MAX + 1]; + void *raw_kind_dsa_area[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; @@ -568,12 +576,12 @@ typedef struct PgStat_ShmemControl pg_atomic_uint64 gc_request_count; /* - * Counters for the number of entries associated to a single stats kind - * that uses variable-numbered objects stored in the shared hash table. - * These counters can be enabled on a per-kind basis, when - * track_entry_count is set. This counter is incremented each time a new - * entry is created (not reused) in the shared hash table, and is - * decremented each time an entry is freed from the shared hash table. + * Counters for the number of entries associated to a single + * variable-numbered stats kind. These counters can be enabled on a + * per-kind basis, when track_entry_count is set. This counter is + * incremented each time a new entry is created (not reused) in a stats + * hashtable, and is decremented each time an entry is freed from a stats + * hashtable. */ pg_atomic_uint64 entry_counts[PGSTAT_KIND_MAX]; @@ -643,8 +651,11 @@ typedef struct PgStat_Snapshot 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; @@ -839,8 +850,9 @@ extern void pgstat_reset_matching_entries(bool (*do_reset) (PgStatShared_HashEnt TimestampTz ts); extern void pgstat_request_entry_refs_gc(void); -extern dsa_pointer pgstat_alloc_entry_body(PgStat_Kind kind); +extern dsa_pointer pgstat_alloc_entry_body(PgStat_Kind kind, dsa_area *dsa); extern PgStatShared_Common *pgstat_init_entry(PgStat_Kind kind, + dsa_area *dsa, PgStatShared_HashEntry *shhashent, dsa_pointer chunk); @@ -1072,4 +1084,15 @@ pgstat_get_custom_snapshot_data(PgStat_Kind kind) return pgStatLocal.snapshot.custom_data[idx]; } +/* + * Returns the dshash table for a given variable-numbered stats kind. + */ +static inline dshash_table * +pgstat_get_hash_for_kind(PgStat_Kind kind) +{ + Assert(pgStatLocal.kind_hash[kind] != NULL); + + return pgStatLocal.kind_hash[kind]; +} + #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 69f2284229e..be494ae3982 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', @@ -28,124 +32,176 @@ $node->safe_psql('postgres', q(CREATE EXTENSION test_custom_var_stats)); $node->safe_psql('postgres', q(CREATE EXTENSION test_custom_fixed_stats)); # Verify custom stats kinds appear in pg_stat_kind_info. -my $result = $node->safe_psql( +$result = $node->safe_psql( 'postgres', q(SELECT id, name, builtin, fixed_amount, accessed_across_databases, - write_to_file + write_to_file, own_hash FROM pg_stat_kind_info WHERE name LIKE 'test_custom%' ORDER BY id)); is( $result, - qq{25|test_custom_var_stats|f|f|t|t -26|test_custom_fixed_stats|f|t|f|t}, + qq{25|test_custom_var_stats|f|f|t|t|f +26|test_custom_fixed_stats|f|t|f|t|f}, "custom stats kinds visible in pg_stat_kind_info"); -# 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 own_hash FROM pg_stat_kind_info + WHERE name = 'test_custom_var_stats')); + is( $result, + $use_own_hash eq 'true' ? 't' : 'f', + "pg_stat_kind_info reports own_hash=$use_own_hash"); + + $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"); + + # Test persistence across clean restart. + $node->stop(); + $node->start(); + + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry1'))); + is( $result, + "entry1|2|Test entry 1", + "variable-sized stats persist after clean restart (own_hash=$use_own_hash)"); + + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry2'))); + is( $result, + "entry2|3|Test entry 2", + "variable-sized stats persist after clean restart (own_hash=$use_own_hash)"); + + # Test reset of variable-sized stats. + $node->safe_psql('postgres', q(select test_custom_stats_var_reset())); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry1'))); + is( $result, + "entry1|0|Test entry 1", + "variable-sized stats reset (own_hash=$use_own_hash)"); + + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry2'))); + is( $result, + "entry2|0|Test entry 2", + "variable-sized stats reset (own_hash=$use_own_hash)"); + + # Test loss after crash recovery. + $node->stop('immediate'); + $node->start; + + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry1'))); + is( $result, + "", + "variable-sized stats of entry1 lost after crash recovery (own_hash=$use_own_hash)"); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry2'))); + is( $result, + "", + "variable-sized stats of entry2 lost after crash recovery (own_hash=$use_own_hash)"); +} + +# 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. -$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"); + q(select numcalls from test_custom_stats_fixed_report())); +is($result, "3", "report for fixed-sized stats"); -# Test persistence across clean restart. +# Test persistence of fixed-sized stats across clean restart. $node->stop(); $node->start(); $result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry1'))); -is( $result, - "entry1|2|Test entry 1", - "variable-sized stats persist after clean restart"); - -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry2'))); -is( $result, - "entry2|3|Test entry 2", - "variable-sized stats persist after clean restart"); - -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_fixed_report())); -is($result, "3|", "fixed-sized stats persist after clean restart"); + q(select numcalls from test_custom_stats_fixed_report())); +is($result, "3", "fixed-sized stats persist after clean restart"); -# Test persistence after crash recovery. +# Test fixed-sized stats after crash recovery. $node->stop('immediate'); $node->start; -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry1'))); -is($result, "", "variable-sized stats of entry1 lost after crash recovery"); -$result = $node->safe_psql('postgres', - q(select * from test_custom_stats_var_report('entry2'))); -is($result, "", "variable-sized stats of entry2 lost after crash recovery"); - # Crash recovery sets the reset timestamp. $result = $node->safe_psql('postgres', q(select numcalls from test_custom_stats_fixed_report() where stats_reset is not null) 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..87cb4118b8b 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 @@ -18,9 +18,19 @@ RETURNS void AS 'MODULE_PATHNAME', 'test_custom_stats_var_drop' LANGUAGE C STRICT PARALLEL UNSAFE; +CREATE FUNCTION test_custom_stats_var_reset() +RETURNS void +AS 'MODULE_PATHNAME', 'test_custom_stats_var_reset' +LANGUAGE C STRICT PARALLEL UNSAFE; + CREATE FUNCTION test_custom_stats_var_report(INOUT name TEXT, OUT calls BIGINT, OUT description TEXT) RETURNS SETOF record AS 'MODULE_PATHNAME', 'test_custom_stats_var_report' 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 ec06d5e1fdb..eb9b1919269 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 @@ -18,6 +18,7 @@ #include "storage/dsm_registry.h" #include "storage/fd.h" #include "utils/builtins.h" +#include "utils/guc.h" #include "utils/pgstat_internal.h" PG_MODULE_MAGIC_EXT( @@ -111,7 +112,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 */ @@ -135,8 +156,25 @@ static const PgStat_KindInfo custom_stats = { void _PG_init(void) { - /* 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); } /*-------------------------------------------------------------------------- @@ -615,6 +653,19 @@ test_custom_stats_var_drop(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* + * test_custom_stats_var_reset + * Reset all custom statistic entries + */ +PG_FUNCTION_INFO_V1(test_custom_stats_var_reset); +Datum +test_custom_stats_var_reset(PG_FUNCTION_ARGS) +{ + pgstat_reset_of_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS); + + PG_RETURN_VOID(); +} + /* * test_custom_stats_var_report * Retrieve custom statistic values @@ -701,3 +752,28 @@ test_custom_stats_var_report(PG_FUNCTION_ARGS) SRF_RETURN_DONE(funcctx); } + +/* + * 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); +} diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 0addd043e68..4addeba2508 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1973,8 +1973,9 @@ pg_stat_kind_info| SELECT id, fixed_amount, accessed_across_databases, write_to_file, + own_hash, entry_count - FROM pg_stat_get_kind_info() k(id, name, builtin, fixed_amount, accessed_across_databases, write_to_file, entry_count); + FROM pg_stat_get_kind_info() k(id, name, builtin, fixed_amount, accessed_across_databases, write_to_file, own_hash, entry_count); pg_stat_lock| SELECT locktype, waits, wait_time, -- 2.50.1