diff options
| author | Peter Eisentraut <peter_e@gmx.net> | 2017-04-10 15:08:14 -0400 |
|---|---|---|
| committer | Peter Eisentraut <peter_e@gmx.net> | 2017-04-10 15:08:14 -0400 |
| commit | 521fd4795e3ec3d0b263b62e5eb58e1557be9c86 (patch) | |
| tree | be08cb0716ac427b4037ea971365244438d22c70 | |
| parent | c45b1d2283c6631b990de25098ea389a1b96277a (diff) | |
| download | postgresql-521fd4795e3ec3d0b263b62e5eb58e1557be9c86.tar.gz postgresql-521fd4795e3ec3d0b263b62e5eb58e1557be9c86.zip | |
Use weaker locks when updating pg_subscription_rel
The previously used ShareRowExclusiveLock, while technically probably
more correct, led to deadlocks during seemingly unrelated operations and
thus a poor experience. Use RowExclusiveLock, like for most similar
catalog operations. In some care cases, the user might see an error
from DDL commands.
Discussion: https://www.postgresql.org/message-id/flat/13592.1490851519%40sss.pgh.pa.us
Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
| -rw-r--r-- | src/backend/catalog/pg_subscription.c | 12 |
1 files changed, 7 insertions, 5 deletions
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index e420ec14d23..f5ba9f6d13c 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -214,6 +214,10 @@ textarray_to_stringlist(ArrayType *textarray) /* * Set the state of a subscription table. + * + * The insert-or-update logic in this function is not concurrency safe so it + * might raise an error in rare circumstances. But if we took a stronger lock + * such as ShareRowExclusiveLock, we would risk more deadlocks. */ Oid SetSubscriptionRelState(Oid subid, Oid relid, char state, @@ -225,8 +229,7 @@ SetSubscriptionRelState(Oid subid, Oid relid, char state, bool nulls[Natts_pg_subscription_rel]; Datum values[Natts_pg_subscription_rel]; - /* Prevent concurrent changes. */ - rel = heap_open(SubscriptionRelRelationId, ShareRowExclusiveLock); + rel = heap_open(SubscriptionRelRelationId, RowExclusiveLock); /* Try finding existing mapping. */ tup = SearchSysCacheCopy2(SUBSCRIPTIONRELMAP, @@ -357,8 +360,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid) HeapTuple tup; int nkeys = 0; - /* Prevent concurrent changes (see SetSubscriptionRelState()). */ - rel = heap_open(SubscriptionRelRelationId, ShareRowExclusiveLock); + rel = heap_open(SubscriptionRelRelationId, RowExclusiveLock); if (OidIsValid(subid)) { @@ -386,7 +388,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid) } heap_endscan(scan); - heap_close(rel, ShareRowExclusiveLock); + heap_close(rel, RowExclusiveLock); } |