From 6ce371bb4547c2967f17f044d6d27654333e837e Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Wed, 14 Oct 2026 14:17:20 -0500 Subject: [PATCH v1 2/2] Fix crash at startup with stats file entry of fixed-numbered stats kind pgstat_read_statsfile() did not check that the kind of a hash entry is variable-numbered. Since 44539786b829, fixed-numbered kinds have no hash table and no DSA area, so such an entry crashes the startup process, and again on each restart as the stats file is not removed. This can happen if a custom kind ID is reused by a fixed-numbered kind. This commit discards the stats file in this case, like fixed-numbered entries already do for variable-numbered kinds. Entries identified by name do not need this check, as fixed-numbered kinds cannot have a from_serialized_name callback, so these entries are already rejected. A GUC is added to test_custom_fixed_stats to set its kind ID, for test coverage. Oversight in 44539786b829. Reported-by: Zsolt Parragi Author: Sami Imseih Author: Zsolt Parragi Discussion: https://postgr.es/m/CAN4CZFMxuOOuYV14+i3YcPG-16q02eBtSVa82J111=ZnDwSicA@mail.gmail.com --- src/backend/utils/activity/pgstat.c | 8 +++++ .../test_custom_stats/t/001_custom_stats.pl | 32 +++++++++++++++++ .../test_custom_fixed_stats.c | 34 +++++++++++++------ 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 620d4d79a46..d31c0b48a1d 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -2052,6 +2052,14 @@ pgstat_read_statsfile(void) key.objid, t); goto error; } + + if (kind_info->fixed_amount) + { + elog(WARNING, "invalid fixed_amount in stats kind for entry %u/%u/%" PRIu64 " of type %c", + key.kind, key.dboid, + key.objid, t); + goto error; + } } else { 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 88dce8caffb..14266b61004 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 @@ -9,6 +9,7 @@ # - Loss after crash recovery. # - Resets for fixed-sized stats. # - Custom hash tables for variable-sized stats +# - Stats file with entries of a kind ID now used by a fixed-sized kind use strict; use warnings FATAL => 'all'; @@ -216,5 +217,36 @@ $result = $node->safe_psql('postgres', ); is($result, "0", "report of fixed-sized after manual reset"); +# Check that entries of a variable-sized stats kind in the stats file are +# discarded if their kind ID is used by a fixed-sized stats kind at the +# next startup. Load only the variable-sized kind first, so as the stats +# file includes only entries for it. +$node->append_conf('postgresql.conf', "test_custom_fixed_stats.kind_id = 25"); +$node->adjust_conf('postgresql.conf', 'shared_preload_libraries', + "'test_custom_var_stats'"); +$node->adjust_conf('postgresql.conf', 'test_custom_var_stats.kind', + "'test_custom_var_stats'"); +$node->restart; + +$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_update('entry1'))); +$node->stop; + +# Use the ID of the variable-sized kind for the fixed-sized kind. +$node->adjust_conf('postgresql.conf', 'shared_preload_libraries', + "'test_custom_fixed_stats'"); +my $log_offset = -s $node->logfile; +$node->start; + +ok( $node->log_contains( + qr/invalid fixed_amount in stats kind for entry 25\/\d+\/\d+ of type S/, + $log_offset), + "stats file discarded with kind 25 now fixed-sized"); + +$result = $node->safe_psql('postgres', + q(select numcalls from test_custom_stats_fixed_report())); +is($result, "0", "fixed-sized stats usable with kind 25"); + # Test completed successfully done_testing(); diff --git a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c index 32be1d587a1..d3829991ec5 100644 --- a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c +++ b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c @@ -17,6 +17,7 @@ #include "funcapi.h" #include "pgstat.h" #include "utils/builtins.h" +#include "utils/guc.h" #include "utils/pgstat_internal.h" #include "utils/timestamp.h" @@ -60,9 +61,11 @@ static const PgStat_KindInfo custom_stats = { }; /* - * Kind ID for test_custom_fixed_stats. + * Kind ID for test_custom_fixed_stats. This is configurable so that tests + * can register it with a different ID, including the one used by + * test_custom_var_stats. */ -#define PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS 26 +static int test_custom_fixed_stats_kind = 26; /*-------------------------------------------------------------------------- * Module initialization @@ -72,8 +75,19 @@ static const PgStat_KindInfo custom_stats = { void _PG_init(void) { + DefineCustomIntVariable("test_custom_fixed_stats.kind_id", + "Sets the ID of the custom fixed stats kind.", + NULL, + &test_custom_fixed_stats_kind, + 26, + PGSTAT_KIND_CUSTOM_MIN, + PGSTAT_KIND_CUSTOM_MAX, + PGC_POSTMASTER, + 0, + NULL, NULL, NULL); + /* Register custom statistics kind */ - pgstat_register_kind(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS, &custom_stats); + pgstat_register_kind(test_custom_fixed_stats_kind, &custom_stats); } /* @@ -97,7 +111,7 @@ static void test_custom_stats_fixed_reset_all_cb(TimestampTz ts) { PgStatShared_CustomFixedEntry *stats_shmem = - pgstat_get_custom_shmem_data(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS); + pgstat_get_custom_shmem_data(test_custom_fixed_stats_kind); /* see explanation above PgStatShared_Archiver for the reset protocol */ LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); @@ -117,9 +131,9 @@ static void test_custom_stats_fixed_snapshot_cb(void) { PgStatShared_CustomFixedEntry *stats_shmem = - pgstat_get_custom_shmem_data(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS); + pgstat_get_custom_shmem_data(test_custom_fixed_stats_kind); PgStat_StatCustomFixedEntry *stat_snap = - pgstat_get_custom_snapshot_data(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS); + pgstat_get_custom_snapshot_data(test_custom_fixed_stats_kind); PgStat_StatCustomFixedEntry *reset_offset = &stats_shmem->reset_offset; PgStat_StatCustomFixedEntry reset; @@ -153,7 +167,7 @@ test_custom_stats_fixed_update(PG_FUNCTION_ARGS) { PgStatShared_CustomFixedEntry *stats_shmem; - stats_shmem = pgstat_get_custom_shmem_data(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS); + stats_shmem = pgstat_get_custom_shmem_data(test_custom_fixed_stats_kind); LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); @@ -174,7 +188,7 @@ PG_FUNCTION_INFO_V1(test_custom_stats_fixed_reset); Datum test_custom_stats_fixed_reset(PG_FUNCTION_ARGS) { - pgstat_reset_of_kind(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS); + pgstat_reset_of_kind(test_custom_fixed_stats_kind); PG_RETURN_VOID(); } @@ -193,8 +207,8 @@ test_custom_stats_fixed_report(PG_FUNCTION_ARGS) PgStat_StatCustomFixedEntry *stats; /* Take snapshot (applies reset offsets) */ - pgstat_snapshot_fixed(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS); - stats = pgstat_get_custom_snapshot_data(PGSTAT_KIND_TEST_CUSTOM_FIXED_STATS); + pgstat_snapshot_fixed(test_custom_fixed_stats_kind); + stats = pgstat_get_custom_snapshot_data(test_custom_fixed_stats_kind); /* Build return tuple */ tupdesc = CreateTemplateTupleDesc(2); -- 2.54.0 (Apple Git-157)