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>

diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index e420ec14d238463fa0f31865ba54513f983de1d3..f5ba9f6d13c9a6f3995c943788574811359bc550 100644 (file)
--- 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);
 }