public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Ayush Tiwari <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Andres Freund <[email protected]>
Cc: Sami Imseih <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Subject: Re: BUG #19520: PANIC when concurrently manipulating stored procedures with pg_stat_statements and track_functions =
Date: Wed, 17 Jun 2026 13:15:13 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAJTYsWVqtWF+KJUdoKrzUM9QPQA5qD225AoeBeN7cwT4V1qd6A@mail.gmail.com>
<[email protected]>
On Tue, Jun 16, 2026 at 08:24:51AM +0900, Michael Paquier wrote:
> On Mon, Jun 15, 2026 at 02:44:06PM +0530, Ayush Tiwari wrote:
>> I've added Andres and Michael on the thread, since they have worked on
>> this in the past, for their input.
>
> Thanks for the poke. I have marked this thread as something to look
> at, but was not able to get back to it. Will investigate..
As far as I can see, pgss is not really a requirement. Your case is
taking advantage of the module introducing more slowness to enlarge
the reproduction window. Now saying that pgss being slow is a good
thing, it's bad, but it helps here. I've tried to reproduce in three
environments, only my mac is able to get something, because it's
slower I guess..
Attached is a script able to reproduce the issue in bash, courtesy of
Claude because java and I sum up to a value very close to 0, see
test_bug19520.txt. The trick of the script is the same as your
scenario, with two concurrent workloads:
- One with DROP PROC/CREATE PROC/CALL.
- One with CALL
I had much more success after adding two sleeps to enlarge the
conflict window, see also the sleep.patch attached, for reference.
Finally attached is a patch, where I'd like to propose the
introduction of a path in pgstat_drop_entry() to make the routine able
to accept double drops.
The big comment within pgstat_init_function_usage() documents why it
does its stuff for track_functions, so I was wondering if we should
enforce the same double-drop-acceptance rule for all the callers
everybody, but I also see a point in the correctness, by allowing the
caller to complain if we try to do double drops but error on them,
pointing to a programming error. Note that
pgstat_drop_entry_internal() is not touched on purpose, to keep the
database-level scans as they are, with double-drops forbidden.
This patch is very close to what Sami has posted on his PGSS thread,
v3-0002, using a missing_ok instead of a skip_dropped:
https://www.postgresql.org/message-id/[email protected]...
I didn't suspect that we would need something like that for a
backpatch, but well.
I'm adding Sami in CC in case he wishes to comment on this patch, and
Horiguchi-san as this area of the code concerns him.
Thoughts or comments welcome.
--
Michael
diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c
index d47d05e3d922..eef97a29de7c 100644
--- a/src/backend/utils/activity/pgstat_function.c
+++ b/src/backend/utils/activity/pgstat_function.c
@@ -112,6 +112,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
AcceptInvalidationMessages();
if (!SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(fcinfo->flinfo->fn_oid)))
{
+ pg_usleep(10000); /* 10ms */
pgstat_drop_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId,
fcinfo->flinfo->fn_oid);
ereport(ERROR, errcode(ERRCODE_UNDEFINED_FUNCTION),
diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c
index 5e2d69e62979..70a1e302514f 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -85,6 +85,8 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, bool isCommit)
* Transaction that dropped an object committed. Drop the stats
* too.
*/
+ if (it->kind == PGSTAT_KIND_FUNCTION)
+ pg_usleep(500000); /* 500ms */
if (!pgstat_drop_entry(it->kind, it->dboid, objid))
not_freed_count++;
}
#!/bin/bash
#
# Reproducer for BUG #19520: PANIC when concurrently manipulating
# stored procedures with track_functions = all (or pl).
#
# On an unfixed build this PANICs. On a fixed build the server survives.
set -e
PGPORT="${1:-5432}"
PGDATABASE="postgres"
PGUSER="${USER:-postgres}"
N_DDL=10 # number of DROP/CREATE/CALL workers
N_CALL=20 # number of CALL-only workers
ITERATIONS=3000 # iterations per worker
export PGPORT PGDATABASE PGUSER
echo "=== BUG #19520 reproducer ==="
echo "Port: $PGPORT | DDL workers: $N_DDL | CALL workers: $N_CALL"
echo "Iterations per worker: $ITERATIONS"
echo ""
# Verify the server is reachable
if ! psql -Xc "SELECT 1" > /dev/null 2>&1; then
echo "ERROR: Cannot connect to PostgreSQL on port $PGPORT."
echo "Make sure the server is running with:"
echo " track_functions = 'all' (or 'pl')"
exit 1
fi
# Check track_functions setting
TF=$(psql -XtAc "SHOW track_functions")
if [ "$TF" = "none" ]; then
echo "ERROR: track_functions = 'none'. Must be 'pl' or 'all'."
exit 1
fi
echo "track_functions = '$TF'"
# Create the initial procedure
psql -Xc "CREATE OR REPLACE PROCEDURE proc_race_test()
LANGUAGE plpgsql AS \$\$ BEGIN END; \$\$;" 2>/dev/null || true
PIDS=()
# Generate a SQL script for DDL workers (persistent connection, tight loop)
DDL_SQL=$(mktemp)
for j in $(seq 1 $ITERATIONS); do
cat >> "$DDL_SQL" <<'EOF'
DROP PROCEDURE IF EXISTS proc_race_test;
CREATE OR REPLACE PROCEDURE proc_race_test() LANGUAGE plpgsql AS $$ BEGIN END; $$;
CALL proc_race_test();
EOF
done
# Generate a SQL script for CALL workers (persistent connection, tight loop)
CALL_SQL=$(mktemp)
for j in $(seq 1 $ITERATIONS); do
echo "CALL proc_race_test();" >> "$CALL_SQL"
done
# Launch DDL workers using persistent psql sessions
for i in $(seq 1 $N_DDL); do
psql -X -f "$DDL_SQL" > /dev/null 2>&1 &
PIDS+=($!)
done
# Launch CALL-only workers using persistent psql sessions
for i in $(seq 1 $N_CALL); do
psql -X -f "$CALL_SQL" > /dev/null 2>&1 &
PIDS+=($!)
done
echo "Launched ${#PIDS[@]} background workers (persistent connections). Waiting..."
echo "(On an unfixed build the server will PANIC; on a fixed build it survives.)"
echo ""
# Wait for all workers
for pid in "${PIDS[@]}"; do
wait "$pid" 2>/dev/null || true
done
# Cleanup temp files
rm -f "$DDL_SQL" "$CALL_SQL"
echo ""
echo "All workers finished."
# Check if the server is still alive
if psql -Xc "SELECT 1" > /dev/null 2>&1; then
echo "PASS: Server is still running - no PANIC occurred."
exit 0
else
echo "FAIL: Server is down - likely hit the PANIC."
echo "Check the server log for:"
echo ' PANIC: cannot abort transaction xxx, it was already committed'
exit 1
fi
From 29c29dd0a778e952cdf6f55c7be08a33436515e2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Wed, 17 Jun 2026 12:59:40 +0900
Subject: [PATCH] Fix potential PANICs with concurrent drop of pgstats entries
---
src/include/utils/pgstat_internal.h | 3 ++-
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 | 17 ++++++++++++++++-
src/backend/utils/activity/pgstat_xact.c | 8 ++++----
.../test_custom_stats/test_custom_var_stats.c | 2 +-
7 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index fe463faaf639..3ca4f4548956 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -807,7 +807,8 @@ 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 missing_ok);
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/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index b67da88c7dc2..c4fa14f138fa 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -650,7 +650,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 d47d05e3d922..f0366e139907 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 0d00dd5d93aa..a32b70a03736 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 b8f354c818a0..ae35332a3347 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -1000,13 +1000,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, uint64 objid)
+pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid,
+ bool missing_ok)
{
PgStat_HashKey key = {0};
PgStatShared_HashEntry *shent;
@@ -1031,6 +1034,18 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
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 objid=%" PRIu64,
+ pgstat_get_kind_info(shent->key.kind)->name,
+ shent->key.dboid,
+ shent->key.objid);
+ 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 5e2d69e62979..3e1978775e16 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, true))
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, true))
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, true))
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, true))
not_freed_count++;
}
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 5c4871ed37ca..863d6a524925 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.54.0
Attachments:
[text/plain] sleep.patch (1.2K, ../[email protected]/2-sleep.patch)
download | inline diff:
diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c
index d47d05e3d922..eef97a29de7c 100644
--- a/src/backend/utils/activity/pgstat_function.c
+++ b/src/backend/utils/activity/pgstat_function.c
@@ -112,6 +112,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
AcceptInvalidationMessages();
if (!SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(fcinfo->flinfo->fn_oid)))
{
+ pg_usleep(10000); /* 10ms */
pgstat_drop_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId,
fcinfo->flinfo->fn_oid);
ereport(ERROR, errcode(ERRCODE_UNDEFINED_FUNCTION),
diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c
index 5e2d69e62979..70a1e302514f 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -85,6 +85,8 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, bool isCommit)
* Transaction that dropped an object committed. Drop the stats
* too.
*/
+ if (it->kind == PGSTAT_KIND_FUNCTION)
+ pg_usleep(500000); /* 500ms */
if (!pgstat_drop_entry(it->kind, it->dboid, objid))
not_freed_count++;
}
[text/plain] test_bug19520.txt (2.7K, ../[email protected]/3-test_bug19520.txt)
download | inline:
#!/bin/bash
#
# Reproducer for BUG #19520: PANIC when concurrently manipulating
# stored procedures with track_functions = all (or pl).
#
# On an unfixed build this PANICs. On a fixed build the server survives.
set -e
PGPORT="${1:-5432}"
PGDATABASE="postgres"
PGUSER="${USER:-postgres}"
N_DDL=10 # number of DROP/CREATE/CALL workers
N_CALL=20 # number of CALL-only workers
ITERATIONS=3000 # iterations per worker
export PGPORT PGDATABASE PGUSER
echo "=== BUG #19520 reproducer ==="
echo "Port: $PGPORT | DDL workers: $N_DDL | CALL workers: $N_CALL"
echo "Iterations per worker: $ITERATIONS"
echo ""
# Verify the server is reachable
if ! psql -Xc "SELECT 1" > /dev/null 2>&1; then
echo "ERROR: Cannot connect to PostgreSQL on port $PGPORT."
echo "Make sure the server is running with:"
echo " track_functions = 'all' (or 'pl')"
exit 1
fi
# Check track_functions setting
TF=$(psql -XtAc "SHOW track_functions")
if [ "$TF" = "none" ]; then
echo "ERROR: track_functions = 'none'. Must be 'pl' or 'all'."
exit 1
fi
echo "track_functions = '$TF'"
# Create the initial procedure
psql -Xc "CREATE OR REPLACE PROCEDURE proc_race_test()
LANGUAGE plpgsql AS \$\$ BEGIN END; \$\$;" 2>/dev/null || true
PIDS=()
# Generate a SQL script for DDL workers (persistent connection, tight loop)
DDL_SQL=$(mktemp)
for j in $(seq 1 $ITERATIONS); do
cat >> "$DDL_SQL" <<'EOF'
DROP PROCEDURE IF EXISTS proc_race_test;
CREATE OR REPLACE PROCEDURE proc_race_test() LANGUAGE plpgsql AS $$ BEGIN END; $$;
CALL proc_race_test();
EOF
done
# Generate a SQL script for CALL workers (persistent connection, tight loop)
CALL_SQL=$(mktemp)
for j in $(seq 1 $ITERATIONS); do
echo "CALL proc_race_test();" >> "$CALL_SQL"
done
# Launch DDL workers using persistent psql sessions
for i in $(seq 1 $N_DDL); do
psql -X -f "$DDL_SQL" > /dev/null 2>&1 &
PIDS+=($!)
done
# Launch CALL-only workers using persistent psql sessions
for i in $(seq 1 $N_CALL); do
psql -X -f "$CALL_SQL" > /dev/null 2>&1 &
PIDS+=($!)
done
echo "Launched ${#PIDS[@]} background workers (persistent connections). Waiting..."
echo "(On an unfixed build the server will PANIC; on a fixed build it survives.)"
echo ""
# Wait for all workers
for pid in "${PIDS[@]}"; do
wait "$pid" 2>/dev/null || true
done
# Cleanup temp files
rm -f "$DDL_SQL" "$CALL_SQL"
echo ""
echo "All workers finished."
# Check if the server is still alive
if psql -Xc "SELECT 1" > /dev/null 2>&1; then
echo "PASS: Server is still running - no PANIC occurred."
exit 0
else
echo "FAIL: Server is down - likely hit the PANIC."
echo "Check the server log for:"
echo ' PANIC: cannot abort transaction xxx, it was already committed'
exit 1
fi
[text/plain] 0001-Fix-potential-PANICs-with-concurrent-drop-of-pgstats.patch (7.1K, ../[email protected]/4-0001-Fix-potential-PANICs-with-concurrent-drop-of-pgstats.patch)
download | inline diff:
From 29c29dd0a778e952cdf6f55c7be08a33436515e2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Wed, 17 Jun 2026 12:59:40 +0900
Subject: [PATCH] Fix potential PANICs with concurrent drop of pgstats entries
---
src/include/utils/pgstat_internal.h | 3 ++-
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 | 17 ++++++++++++++++-
src/backend/utils/activity/pgstat_xact.c | 8 ++++----
.../test_custom_stats/test_custom_var_stats.c | 2 +-
7 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index fe463faaf639..3ca4f4548956 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -807,7 +807,8 @@ 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 missing_ok);
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/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index b67da88c7dc2..c4fa14f138fa 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -650,7 +650,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 d47d05e3d922..f0366e139907 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 0d00dd5d93aa..a32b70a03736 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 b8f354c818a0..ae35332a3347 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -1000,13 +1000,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, uint64 objid)
+pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid,
+ bool missing_ok)
{
PgStat_HashKey key = {0};
PgStatShared_HashEntry *shent;
@@ -1031,6 +1034,18 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
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 objid=%" PRIu64,
+ pgstat_get_kind_info(shent->key.kind)->name,
+ shent->key.dboid,
+ shent->key.objid);
+ 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 5e2d69e62979..3e1978775e16 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, true))
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, true))
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, true))
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, true))
not_freed_count++;
}
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 5c4871ed37ca..863d6a524925 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.54.0
[application/pgp-signature] signature.asc (833B, ../[email protected]/5-signature.asc)
download
view thread (17+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19520: PANIC when concurrently manipulating stored procedures with pg_stat_statements and track_functions =
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox