From 181f051a79e565eaed25fcceddc1f2c90adaae1f Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Thu, 28 May 2026 12:02:21 +0000 Subject: [PATCH v3 2/5] pgstat: tolerate already-dropped entries in pgstat_drop_entry() There are valid cases in which pgstat_drop_entry could be called multiple times before a garbage-collection occurs, leading to an ERROR if the entry was already marked dead. A caller should be able to tolerate such cases by supplying a skip_dropped = true argument. This will be needed for future work in which parallel backends may attempt to drop an entry by hash-key. --- src/backend/utils/activity/pgstat.c | 2 +- src/backend/utils/activity/pgstat_function.c | 2 +- src/backend/utils/activity/pgstat_replslot.c | 2 +- src/backend/utils/activity/pgstat_shmem.c | 12 ++++++++++-- src/backend/utils/activity/pgstat_xact.c | 8 ++++---- src/include/utils/pgstat_internal.h | 2 +- .../test_custom_stats/test_custom_var_stats.c | 2 +- 7 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c index 9b5d9bf09cb..e92dbd38e8d 100644 --- a/src/backend/utils/activity/pgstat.c +++ b/src/backend/utils/activity/pgstat.c @@ -651,7 +651,7 @@ pgstat_shutdown_hook(int code, Datum arg) dlist_init(&pgStatPending); /* drop the backend stats entry */ - if (!pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber)) + if (!pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber, false)) pgstat_request_entry_refs_gc(); pgstat_detach_shmem(); diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c index d47d05e3d92..88ac6d08e69 100644 --- a/src/backend/utils/activity/pgstat_function.c +++ b/src/backend/utils/activity/pgstat_function.c @@ -113,7 +113,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo, if (!SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(fcinfo->flinfo->fn_oid))) { pgstat_drop_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId, - fcinfo->flinfo->fn_oid); + fcinfo->flinfo->fn_oid, false); ereport(ERROR, errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("function call to dropped function")); } diff --git a/src/backend/utils/activity/pgstat_replslot.c b/src/backend/utils/activity/pgstat_replslot.c index 0d00dd5d93a..a32b70a0373 100644 --- a/src/backend/utils/activity/pgstat_replslot.c +++ b/src/backend/utils/activity/pgstat_replslot.c @@ -188,7 +188,7 @@ pgstat_drop_replslot(ReplicationSlot *slot) Assert(LWLockHeldByMeInMode(ReplicationSlotAllocationLock, LW_EXCLUSIVE)); if (!pgstat_drop_entry(PGSTAT_KIND_REPLSLOT, InvalidOid, - ReplicationSlotIndex(slot))) + ReplicationSlotIndex(slot), false)) pgstat_request_entry_refs_gc(); } diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index b8f354c818a..403e11a0da8 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -998,7 +998,8 @@ pgstat_drop_database_and_contents(Oid dboid) * Drop a single stats entry. * * This routine returns false if the stats entry of the dropped object could - * not be freed, true otherwise. + * not be freed, true otherwise. If skip_dropped is true, already-dropped + * entries are tolerated and treated as success. * * The callers of this function should call pgstat_request_entry_refs_gc() * if the stats entry could not be freed, to ensure that this entry's memory @@ -1006,7 +1007,7 @@ pgstat_drop_database_and_contents(Oid dboid) * pgstat_gc_entry_refs(). */ bool -pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid) +pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool skip_dropped) { PgStat_HashKey key = {0}; PgStatShared_HashEntry *shent; @@ -1031,6 +1032,13 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid) shent = dshash_find(pgStatLocal.shared_hash, &key, true); if (shent) { + /* already dropped by another backend, nothing to do */ + if (shent->dropped && skip_dropped) + { + dshash_release_lock(pgStatLocal.shared_hash, shent); + return true; + } + freed = pgstat_drop_entry_internal(shent, NULL); /* diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c index 5e2d69e6297..927b6268fc5 100644 --- a/src/backend/utils/activity/pgstat_xact.c +++ b/src/backend/utils/activity/pgstat_xact.c @@ -85,7 +85,7 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, bool isCommit) * Transaction that dropped an object committed. Drop the stats * too. */ - if (!pgstat_drop_entry(it->kind, it->dboid, objid)) + if (!pgstat_drop_entry(it->kind, it->dboid, objid, false)) not_freed_count++; } else if (!isCommit && pending->is_create) @@ -94,7 +94,7 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, bool isCommit) * Transaction that created an object aborted. Drop the stats * associated with the object. */ - if (!pgstat_drop_entry(it->kind, it->dboid, objid)) + if (!pgstat_drop_entry(it->kind, it->dboid, objid, false)) not_freed_count++; } @@ -160,7 +160,7 @@ AtEOSubXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, * Subtransaction creating a new stats object aborted. Drop the * stats object. */ - if (!pgstat_drop_entry(it->kind, it->dboid, objid)) + if (!pgstat_drop_entry(it->kind, it->dboid, objid, false)) not_freed_count++; pfree(pending); } @@ -323,7 +323,7 @@ pgstat_execute_transactional_drops(int ndrops, struct xl_xact_stats_item *items, xl_xact_stats_item *it = &items[i]; uint64 objid = ((uint64) it->objid_hi) << 32 | it->objid_lo; - if (!pgstat_drop_entry(it->kind, it->dboid, objid)) + if (!pgstat_drop_entry(it->kind, it->dboid, objid, false)) not_freed_count++; } diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index fe463faaf63..96f189e0dc3 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -807,7 +807,7 @@ extern PgStat_EntryRef *pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, uint64 extern bool pgstat_lock_entry(PgStat_EntryRef *entry_ref, bool nowait); extern bool pgstat_lock_entry_shared(PgStat_EntryRef *entry_ref, bool nowait); extern void pgstat_unlock_entry(PgStat_EntryRef *entry_ref); -extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid); +extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool skip_dropped); extern void pgstat_drop_all_entries(void); extern void pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), Datum match_data); 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 5c4871ed37c..863d6a52492 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 @@ -600,7 +600,7 @@ test_custom_stats_var_drop(PG_FUNCTION_ARGS) /* Drop entry and request GC if the entry could not be freed */ if (!pgstat_drop_entry(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, InvalidOid, - PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name))) + PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name), false)) pgstat_request_entry_refs_gc(); PG_RETURN_VOID(); -- 2.50.1 (Apple Git-155)