Commit 521fd479 authored by Peter Eisentraut's avatar Peter Eisentraut

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>
parent c45b1d22
...@@ -214,6 +214,10 @@ textarray_to_stringlist(ArrayType *textarray) ...@@ -214,6 +214,10 @@ textarray_to_stringlist(ArrayType *textarray)
/* /*
* Set the state of a subscription table. * 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 Oid
SetSubscriptionRelState(Oid subid, Oid relid, char state, SetSubscriptionRelState(Oid subid, Oid relid, char state,
...@@ -225,8 +229,7 @@ SetSubscriptionRelState(Oid subid, Oid relid, char state, ...@@ -225,8 +229,7 @@ SetSubscriptionRelState(Oid subid, Oid relid, char state,
bool nulls[Natts_pg_subscription_rel]; bool nulls[Natts_pg_subscription_rel];
Datum values[Natts_pg_subscription_rel]; Datum values[Natts_pg_subscription_rel];
/* Prevent concurrent changes. */ rel = heap_open(SubscriptionRelRelationId, RowExclusiveLock);
rel = heap_open(SubscriptionRelRelationId, ShareRowExclusiveLock);
/* Try finding existing mapping. */ /* Try finding existing mapping. */
tup = SearchSysCacheCopy2(SUBSCRIPTIONRELMAP, tup = SearchSysCacheCopy2(SUBSCRIPTIONRELMAP,
...@@ -357,8 +360,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid) ...@@ -357,8 +360,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid)
HeapTuple tup; HeapTuple tup;
int nkeys = 0; int nkeys = 0;
/* Prevent concurrent changes (see SetSubscriptionRelState()). */ rel = heap_open(SubscriptionRelRelationId, RowExclusiveLock);
rel = heap_open(SubscriptionRelRelationId, ShareRowExclusiveLock);
if (OidIsValid(subid)) if (OidIsValid(subid))
{ {
...@@ -386,7 +388,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid) ...@@ -386,7 +388,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid)
} }
heap_endscan(scan); heap_endscan(scan);
heap_close(rel, ShareRowExclusiveLock); heap_close(rel, RowExclusiveLock);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment