Re: [HACKERS] DROP SUBSCRIPTION hangs if sub is disabled in the same transaction

Arseny Sher <a.sher@postgrespro.ru> writes:

> Attached patch fixes this by stopping workers before RO drop, as
> already done in case when we drop replication slot.

Sorry, here is the patch.
From 008d54dfe2e8ba610bb7c897cfbb4ee7a700aa2e Mon Sep 17 00:00:00 2001
From: Arseny Sher <sher-ars@yandex.ru>
Date: Mon, 4 Sep 2017 16:55:08 +0300
Subject: [PATCH] Stop LR workers before dropping replication origin.

Previously, executing ALTER SUBSCRIPTION DISABLE and DROP SUBSCRIPTION in one
transaction would hang forever, because worker didn't stop until transaction is
commited, and transaction couldn't commit before worker exit since the latter
occupies ReplicationState.
---
 src/backend/commands/subscriptioncmds.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 77620dfd42..5d608855a1 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -886,6 +886,10 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
     else
         slotname = NULL;
 
+    /* Get originid */
+    snprintf(originname, sizeof(originname), "pg_%u", subid);
+    originid = replorigin_by_name(originname, true);
+
     /*
      * Since dropping a replication slot is not transactional, the replication
      * slot stays dropped even if the transaction rolls back.  So we cannot
@@ -909,9 +913,10 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
     ReleaseSysCache(tup);
 
     /*
-     * If we are dropping the replication slot, stop all the subscription
-     * workers immediately, so that the slot becomes accessible.  Otherwise
-     * just schedule the stopping for the end of the transaction.
+     * We stop all subscription workers immediately in two cases:
+     * - We are dropping the replication slot, to make the slot accessible;
+     * - We are dropping repl origin tracking, to release ReplicationState;
+     * Otherwise just schedule the stopping for the end of the transaction.
      *
      * New workers won't be started because we hold an exclusive lock on the
      * subscription till the end of the transaction.
@@ -923,7 +928,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
     {
         LogicalRepWorker *w = (LogicalRepWorker *) lfirst(lc);
 
-        if (slotname)
+        if (slotname || originid != InvalidRepOriginId)
             logicalrep_worker_stop(w->subid, w->relid);
         else
             logicalrep_worker_stop_at_commit(w->subid, w->relid);
@@ -937,8 +942,6 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
     RemoveSubscriptionRel(subid, InvalidOid);
 
     /* Remove the origin tracking if exists. */
-    snprintf(originname, sizeof(originname), "pg_%u", subid);
-    originid = replorigin_by_name(originname, true);
     if (originid != InvalidRepOriginId)
         replorigin_drop(originid, false);
 
-- 
2.11.0



--
Arseny Sher
Postgres Professional: https://postgrespro.ru
The Russian Postgres Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers