v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v15.patch

From 1ecb7c1a89872b32b83c25b7fba6b1f07cbc9d33 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 14:54:31 +0000
Subject: [PATCH v5] Fix DROP SUBSCRIPTION deadlock with new database creation

DROP SUBSCRIPTION previously acquired an AccessExclusiveLock on the
pg_subscription catalog to prevent the replication launcher from starting
a new worker. However, this caused a deadlock. New database creation also
need to acquire an AccessShareLock on this same catalog during their
initialization phase. This created a lock conflict where DROP SUBSCRIPTION
would block this, while simultaneously waiting for a connection to
complete for dropping the replication slot.

This commit resolves the deadlock by having DROP SUBSCRIPTION acquire a less
restrictive RowExclusiveLock on the catalog instead.

To address the original concern of orphaned workers, a new check is
implemented.  The replication worker now takes a shared object lock on the
subscription itself.

If a worker starts for a subscription that no longer exists, it immediately detects
this condition and exits. This ensures that no orphan workers are created without
the need for an overly broad AccessExclusiveLock on the system catalog.
---
 src/backend/commands/subscriptioncmds.c    |  8 +++++---
 src/backend/replication/logical/worker.c   |  7 +++++++
 src/bin/pg_upgrade/t/002_pg_upgrade.pl     |  2 ++
 src/test/regress/expected/subscription.out | 12 ++++++++++++
 src/test/regress/sql/subscription.sql      | 13 +++++++++++++
 5 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 334717c0e96..cb9867c96d4 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1341,10 +1341,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
 	List	   *rstates;
 
 	/*
-	 * Lock pg_subscription with AccessExclusiveLock to ensure that the
-	 * launcher doesn't restart new worker during dropping the subscription
+	 * The launcher may concurrently start a new worker for this subscription.
+	 * During initialization, the worker checks for subscription validity and
+	 * exits if the subscription has already been dropped. See
+	 * InitializeLogRepWorker.
 	 */
-	rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+	rel = table_open(SubscriptionRelationId, RowExclusiveLock);
 
 	tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
 						  CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 155bf7c44ee..2c4bc62341f 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3638,6 +3638,13 @@ ApplyWorkerMain(Datum main_arg)
 	StartTransactionCommand();
 	oldctx = MemoryContextSwitchTo(ApplyContext);
 
+	/*
+	 * Lock the subscription to prevent it from being concurrently dropped,
+	 * then re-verify its existence. After the initialization, the worker will
+	 * be terminated gracefully if the subscription is dropped.
+	 */
+	LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+					 AccessShareLock);
 	MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
 	if (!MySubscription)
 	{
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 959c158be90..f41fb17da85 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -92,6 +92,8 @@ my %node_params = ();
 $node_params{extra} = [ '--wal-segsize', '1', '--allow-group-access' ]
   if $oldnode->pg_version >= 11;
 $oldnode->init(%node_params);
+$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
+$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
 $oldnode->start;
 
 # The default location of the source code is the root of this directory.
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index ab587354755..85259fedc0f 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -347,6 +347,18 @@ ALTER SUBSCRIPTION regress_testsub SET (disable_on_error = true);
 
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
 DROP SUBSCRIPTION regress_testsub;
+-- this test verifies that DROP SUBSCRIPTION returns an immediate error
+-- and does not hang when it makes connection to a recently created database
+-- whose caches are not yet initialized.
+CREATE DATABASE regression_db;
+CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regression_db' PUBLICATION testpub WITH (connect=false);
+WARNING:  tables were not subscribed, you will have to run ALTER SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables
+DROP SUBSCRIPTION regress_testsub;
+ERROR:  could not drop replication slot "regress_testsub" on publisher: ERROR:  replication slot "regress_testsub" does not exist
+-- now set the slot_name to NONE and DROP SUBSCRIPTION, it should work
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
+DROP SUBSCRIPTION regress_testsub;
+DROP DATABASE regression_db;
 RESET SESSION AUTHORIZATION;
 DROP ROLE regress_subscription_user;
 DROP ROLE regress_subscription_user2;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 7c5e7487bc7..d8be0869678 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -261,6 +261,19 @@ ALTER SUBSCRIPTION regress_testsub SET (disable_on_error = true);
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
 DROP SUBSCRIPTION regress_testsub;
 
+-- this test verifies that DROP SUBSCRIPTION returns an immediate error
+-- and does not hang when it makes connection to a recently created database
+-- whose caches are not yet initialized.
+CREATE DATABASE regression_db;
+CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regression_db' PUBLICATION testpub WITH (connect=false);
+
+DROP SUBSCRIPTION regress_testsub;
+
+-- now set the slot_name to NONE and DROP SUBSCRIPTION, it should work
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
+DROP SUBSCRIPTION regress_testsub;
+DROP DATABASE regression_db;
+
 RESET SESSION AUTHORIZATION;
 DROP ROLE regress_subscription_user;
 DROP ROLE regress_subscription_user2;
-- 
2.51.0.rc1.163.g2494970778-goog