postgres.git / summary / log / commit / refs
commit 1e9e62193c3fa66e39b50163f225388fcf6eb7d5
Author: Michael Paquier <michael@paquier.xyz>
Date: Thu Jun 18 02:49:41 2026 +0000
Fix PANIC with track_functions due to concurrent drop of pgstats entries
pgstat_drop_entry_internal() generates an ERROR if facing a pgstats
entry already marked as dropped. With a workload doing a lot of
concurrent CALL and DROP/CREATE PROCEDURE, it could be possible for
AtEOXact_PgStat_DroppedStats(), that wants to do transactional drops, to
find entries that are already dropped, after a commit record has been
written. In this case, ERRORs are upgraded to PANIC, taking down the
server.
This issue is fixed by making pgstat_drop_entry() optionally more
tolerant to concurrent drops, adding to the routine a missing_ok option
to make some of its callers more tolerant (spoiler: some of the callers
want a strict behavior, like replication slots and backend stats).
pgstat_drop_entry_internal() cannot be called anymore for an entry
marked as dropped, hence its error is replaced by an assertion.
Functions are handled as a special case in core; this problem could also
apply to custom stats kinds depending on what an extension does.
track_functions is costly when enabled (disabled by default), which is
perhaps the main reason why this has not be found yet.
A similar version of this patch has been proposed by Sami Imseih on a
different thread for a feature in development. This version has tweaked
here by me for the sake of fixing this issue.
Reported-by: zhanglihui <zlh21343@163.com>
Author: Sami Imseih <samimseih@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/19520-73873648d44793cf@postgresql.org
Backpatch-through: 15
src/backend/utils/activity/pgstat_function.c | 2 +-
src/backend/utils/activity/pgstat_replslot.c | 2 +-
src/backend/utils/activity/pgstat_shmem.c | 27 +++++++++++++++++++--------
src/backend/utils/activity/pgstat_xact.c | 8 ++++----
src/include/utils/pgstat_internal.h | 3 ++-
5 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c
index b2e40d26713..d1853f33753 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, true);
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 4faf06045c5..44047747056 100644
--- a/src/backend/utils/activity/pgstat_replslot.c
+++ b/src/backend/utils/activity/pgstat_replslot.c
@@ -154,7 +154,7 @@ void
pgstat_drop_replslot(ReplicationSlot *slot)
{
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 11262220305..5a4657ed42b 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -857,13 +857,7 @@ pgstat_drop_entry_internal(PgStatShared_HashEntry *shent,
* Signal that the entry is dropped - this will eventually cause other
* backends to release their references.
*/
- if (shent->dropped)
- elog(ERROR,
- "trying to drop stats entry already dropped: kind=%s dboid=%u objoid=%u refcount=%u generation=%u",
- pgstat_get_kind_info(shent->key.kind)->name,
- shent->key.dboid, shent->key.objoid,
- pg_atomic_read_u32(&shent->refcount),
- pg_atomic_read_u32(&shent->generation));
+ Assert(!shent->dropped);
shent->dropped = true;
/* release refcount marking entry as not dropped */
@@ -939,13 +933,16 @@ pgstat_drop_database_and_contents(Oid dboid)
* This routine returns false if the stats entry of the dropped object could
* not be freed, true otherwise.
*
+ * If missing_ok is true, skip entries that have been concurrently dropped.
+ *
* 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
* can be reclaimed later by a different backend calling
* pgstat_gc_entry_refs().
*/
bool
-pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid)
+pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid,
+ bool missing_ok)
{
PgStat_HashKey key;
PgStatShared_HashEntry *shent;
@@ -973,6 +970,20 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid)
shent = dshash_find(pgStatLocal.shared_hash, &key, true);
if (shent)
{
+ if (shent->dropped)
+ {
+ if (!missing_ok)
+ elog(ERROR,
+ "trying to drop stats entry already dropped: kind=%s dboid=%u objoid=%u refcount=%u generation=%u",
+ pgstat_get_kind_info(shent->key.kind)->name,
+ shent->key.dboid,
+ shent->key.objoid,
+ pg_atomic_read_u32(&shent->refcount),
+ pg_atomic_read_u32(&shent->generation));
+ 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 d6f660edf7b..31a73d83865 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -88,7 +88,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, it->objoid))
+ if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true))
not_freed_count++;
}
else if (!isCommit && pending->is_create)
@@ -97,7 +97,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, it->objoid))
+ if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true))
not_freed_count++;
}
@@ -164,7 +164,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, it->objoid))
+ if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true))
not_freed_count++;
pfree(pending);
}
@@ -328,7 +328,7 @@ pgstat_execute_transactional_drops(int ndrops, struct xl_xact_stats_item *items,
{
xl_xact_stats_item *it = &items[i];
- if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid))
+ if (!pgstat_drop_entry(it->kind, it->dboid, it->objoid, true))
not_freed_count++;
}
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index efe5dfc2321..ec9fa891670 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -603,7 +603,8 @@ extern PgStat_EntryRef *pgstat_get_entry_ref(PgStat_Kind kind, Oid dboid, Oid ob
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, Oid objoid);
+extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, Oid objoid,
+ bool missing_ok);
extern void pgstat_drop_all_entries(void);
extern PgStat_EntryRef *pgstat_get_entry_ref_locked(PgStat_Kind kind, Oid dboid, Oid objoid,
bool nowait);
[parent: a6f23c3ade0e]