v3-0002-pgstat-tolerate-already-dropped-entries-in-pgstat.patch

application/x-patch

Filename: v3-0002-pgstat-tolerate-already-dropped-entries-in-pgstat.patch
Type: application/x-patch
Part: 6
Message: Re: Improve pg_stat_statements scalability
From 181f051a79e565eaed25fcceddc1f2c90adaae1f Mon Sep 17 00:00:00 2001
From: Sami Imseih <samimseih@gmail.com>
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)