On 11/8/23 16:52, Tom Lane wrote:
> Tomas Vondra <tomas.von...@enterprisedb.com> writes:
>> On 11/8/23 16:10, Justin Pryzby wrote:
>>> I found this in our logs, and reproduced it under v11-v16.
>>>
>>> CREATE TABLE t(a int, b int);
>>> INSERT INTO t SELECT generate_series(1,999);
>>> CREATE STATISTICS t_stats ON a,b FROM t;
>>>
>>> while :; do psql postgres -qtxc "ANALYZE t"; done &
>>> while :; do psql postgres -qtxc "begin; DROP STATISTICS t_stats"; done &
>>>
>>> It's known that concurrent DDL can hit elog(). But in this case,
>>> there's only one DDL operation.
>
>> AFAICS this happens because store_statext (after ANALYZE builds the new
>> statistics) does this:
>
> Shouldn't DROP STATISTICS be taking a lock on the associated table
> that is strong enough to lock out ANALYZE?
>
Yes, I think that's the correct thing to do. I recall having a
discussion about this with someone while working on the patch, leading
to the current code. But I haven't managed to find that particular bit
in the archives :-(
Anyway, the attached patch should fix this by getting the lock, I think.
- RemoveStatisticsById is what gets called drop DROP STATISTICS (or for
dependencies), so that's where we get the AE lock
- RemoveStatisticsDataById gets called from ANALYZE, so that already
should have a lock (so no need to acquire another one)
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 36bc8c33ba..dae0fd6b4f 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -734,18 +734,11 @@ void
RemoveStatisticsById(Oid statsOid)
{
Relation relation;
+ Relation rel;
HeapTuple tup;
Form_pg_statistic_ext statext;
Oid relid;
- /*
- * First delete the pg_statistic_ext_data tuples holding the actual
- * statistical data. There might be data with/without inheritance, so
- * attempt deleting both.
- */
- RemoveStatisticsDataById(statsOid, true);
- RemoveStatisticsDataById(statsOid, false);
-
/*
* Delete the pg_statistic_ext tuple. Also send out a cache inval on the
* associated table, so that dependent plans will be rebuilt.
@@ -760,12 +753,25 @@ RemoveStatisticsById(Oid statsOid)
statext = (Form_pg_statistic_ext) GETSTRUCT(tup);
relid = statext->stxrelid;
+ rel = table_open(relid, AccessExclusiveLock);
+
+ /*
+ * First delete the pg_statistic_ext_data tuples holding the actual
+ * statistical data. There might be data with/without inheritance, so
+ * attempt deleting both.
+ */
+ RemoveStatisticsDataById(statsOid, true);
+ RemoveStatisticsDataById(statsOid, false);
+
CacheInvalidateRelcacheByRelid(relid);
CatalogTupleDelete(relation, &tup->t_self);
ReleaseSysCache(tup);
+ /* Keep lock on the rel until end of xact */
+ table_close(rel, NoLock);
+
table_close(relation, RowExclusiveLock);
}
