pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
37+ messages / 6 participants
[nested] [flat]
* BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-07-16 19:00 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 37+ messages in thread
From: PG Bug reporting form @ 2025-07-16 19:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
The following bug has been logged on the website:
Bug reference: 18988
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 18beta1
Operating system: Ubuntu 24.04
Description:
The following script:
createdb ndb
echo "
CREATE SUBSCRIPTION testsub CONNECTION 'dbname=ndb' PUBLICATION testpub WITH
(connect = false);
" | psql
echo "
DROP SUBSCRIPTION testsub
" | psql &
sleep 1
timeout 30 psql ndb -c "SELECT 1" || echo "TIMEOUT"
makes DROP SUBSCRIPTION stuck on waiting for a connection to drop a slot,
while this connection is waiting for a lock for relation 6100
(pg_subscription), locked by DROP SUBSCRIPTION:
law 1545967 1545946 0 21:10 ? 00:00:00 postgres: law regression
[local] DROP SUBSCRIPTION
law 1545968 1545946 0 21:10 ? 00:00:00 postgres: walsender law
ndb [local] startup waiting
With debug_discard_caches = 1 or under some lucky circumstances (I
encountered these), this leads to inability to connect to any database.
Reproduced on REL_13_STABLE .. master.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-07-30 10:54 vignesh C <vignesh21@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 37+ messages in thread
From: vignesh C @ 2025-07-30 10:54 UTC (permalink / raw)
To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
On Thu, 17 Jul 2025 at 00:36, PG Bug reporting form
<noreply@postgresql.org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 18988
> Logged by: Alexander Lakhin
> Email address: exclusion@gmail.com
> PostgreSQL version: 18beta1
> Operating system: Ubuntu 24.04
> Description:
>
> The following script:
> createdb ndb
> echo "
> CREATE SUBSCRIPTION testsub CONNECTION 'dbname=ndb' PUBLICATION testpub WITH
> (connect = false);
> " | psql
>
> echo "
> DROP SUBSCRIPTION testsub
> " | psql &
> sleep 1
> timeout 30 psql ndb -c "SELECT 1" || echo "TIMEOUT"
>
> makes DROP SUBSCRIPTION stuck on waiting for a connection to drop a slot,
> while this connection is waiting for a lock for relation 6100
> (pg_subscription), locked by DROP SUBSCRIPTION:
> law 1545967 1545946 0 21:10 ? 00:00:00 postgres: law regression
> [local] DROP SUBSCRIPTION
> law 1545968 1545946 0 21:10 ? 00:00:00 postgres: walsender law
> ndb [local] startup waiting
>
> With debug_discard_caches = 1 or under some lucky circumstances (I
> encountered these), this leads to inability to connect to any database.
>
> Reproduced on REL_13_STABLE .. master.
Thanks, I was able to reproduce the issue using the steps provided.
The problem occurs because: When dropping a subscription, it takes an
AccessExclusiveLock on the pg_subscription system tables to prevent
the launcher from restarting the worker. During this process, it also
attempts to connect to the publisher in order to drop the replication
slot. As we are connecting to a newly created database, it may not yet
have initialized its catalog caches. As part of the backend startup,
it attempts to build the cache hierarchy via:
RelationCacheInitializePhase3 → InitCatalogCachePhase2 →
InitCatCachePhase2 This cache initialization requires acquiring a
shared lock on pg_subscription, since it is one of the syscache-backed
catalog tables. But that shared lock is blocked by the
AccessExclusiveLock already held by the dropping process. As a result,
the new backend hangs waiting for the lock, and the original DROP
SUBSCRIPTION process cannot proceed, leading to a self-blocking
scenario.
In this specific case, no replication slot was created during
subscription creation as the connect option was specified as false.
Therefore, I believe the system should skip connecting to the
publisher when dropping the subscription. I've attached a patch that
addresses this behavior. Thoughts?
Regards,
Vignesh
Attachments:
[application/octet-stream] drop_subscription_wait_issue.patch (6.0K, ../../CALDaNm0wf0XVKmcf=s2e3d52b4VS9wkBao1HC9rXJB=n5Mxs2g@mail.gmail.com/2-drop_subscription_wait_issue.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index b8cd15f3280..0086b5a0e79 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -168,10 +168,10 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
<term><literal>slot_name</literal> (<type>string</type>)</term>
<listitem>
<para>
- Name of the publisher's replication slot to use. The default is
- to use the name of the subscription for the slot name. The name cannot
- be <literal>pg_conflict_detection</literal> as it is reserved for the
- conflict detection.
+ Name of the publisher's replication slot to use. When
+ <literal>connect</literal> is enabled, the slot name defaults to the
+ name of the subscription. The name cannot be <literal>pg_conflict_detection</literal>
+ as it is reserved for the conflict detection.
</para>
<para>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index cd6c3684482..1d5d3b581f4 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -651,7 +651,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
if (opts.retaindeadtuples)
CheckSubDeadTupleRetention(true, !opts.enabled, WARNING);
- if (!IsSet(opts.specified_opts, SUBOPT_SLOT_NAME) &&
+ if (!IsSet(opts.specified_opts, SUBOPT_SLOT_NAME) && opts.connect &&
opts.slot_name == NULL)
opts.slot_name = stmt->subname;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 6c7ec80e271..a7e0df0f1eb 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -3336,7 +3336,7 @@ my %tests = (
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
WITH (connect = false);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1', streaming = parallel);\E
+ \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = NONE, streaming = parallel);\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -3347,7 +3347,7 @@ my %tests = (
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
WITH (connect = false, origin = none, streaming = off);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub2 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub2', streaming = off, origin = none);\E
+ \QCREATE SUBSCRIPTION sub2 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = NONE, streaming = off, origin = none);\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -3358,7 +3358,7 @@ my %tests = (
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
WITH (connect = false, origin = any, streaming = on);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub3 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub3', streaming = on);\E
+ \QCREATE SUBSCRIPTION sub3 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = NONE, streaming = on);\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index a98c97f7616..180be3c2bde 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -343,6 +343,7 @@ CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUB
WITH (connect = false, create_slot = false, copy_data = false);
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'regress_testsub');
ALTER SUBSCRIPTION regress_testsub ENABLE;
-- fail - ALTER SUBSCRIPTION with refresh is not allowed in a transaction
-- block or function
@@ -467,6 +468,7 @@ SET SESSION AUTHORIZATION regress_subscription_user3;
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist password=regress_fakepassword' PUBLICATION testpub WITH (connect = false);
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'regress_testsub');
-- we cannot give the subscription away to some random user
ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user;
ERROR: must be able to SET ROLE "regress_subscription_user"
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index f0f714fe747..7cdd8166b6b 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -228,6 +228,7 @@ DROP SUBSCRIPTION regress_testsub;
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION mypub
WITH (connect = false, create_slot = false, copy_data = false);
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'regress_testsub');
ALTER SUBSCRIPTION regress_testsub ENABLE;
-- fail - ALTER SUBSCRIPTION with refresh is not allowed in a transaction
@@ -321,6 +322,7 @@ RESET SESSION AUTHORIZATION;
GRANT CREATE ON DATABASE REGRESSION TO regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist password=regress_fakepassword' PUBLICATION testpub WITH (connect = false);
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'regress_testsub');
-- we cannot give the subscription away to some random user
ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user;
[application/x-zip-compressed] drop_sub_hang_repro.zip (444B, ../../CALDaNm0wf0XVKmcf=s2e3d52b4VS9wkBao1HC9rXJB=n5Mxs2g@mail.gmail.com/3-drop_sub_hang_repro.zip)
download
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-07-31 05:20 Dilip Kumar <dilipbalaut@gmail.com>
parent: vignesh C <vignesh21@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-07-31 05:20 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
On Wed, Jul 30, 2025 at 4:24 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, 17 Jul 2025 at 00:36, PG Bug reporting form
> <noreply@postgresql.org> wrote:
> >
> > The following bug has been logged on the website:
> >
> > Bug reference: 18988
> > Logged by: Alexander Lakhin
> > Email address: exclusion@gmail.com
> > PostgreSQL version: 18beta1
> > Operating system: Ubuntu 24.04
> > Description:
> >
> > The following script:
> > createdb ndb
> > echo "
> > CREATE SUBSCRIPTION testsub CONNECTION 'dbname=ndb' PUBLICATION testpub WITH
> > (connect = false);
> > " | psql
> >
> > echo "
> > DROP SUBSCRIPTION testsub
> > " | psql &
> > sleep 1
> > timeout 30 psql ndb -c "SELECT 1" || echo "TIMEOUT"
> >
> > makes DROP SUBSCRIPTION stuck on waiting for a connection to drop a slot,
> > while this connection is waiting for a lock for relation 6100
> > (pg_subscription), locked by DROP SUBSCRIPTION:
> > law 1545967 1545946 0 21:10 ? 00:00:00 postgres: law regression
> > [local] DROP SUBSCRIPTION
> > law 1545968 1545946 0 21:10 ? 00:00:00 postgres: walsender law
> > ndb [local] startup waiting
> >
> > With debug_discard_caches = 1 or under some lucky circumstances (I
> > encountered these), this leads to inability to connect to any database.
> >
> > Reproduced on REL_13_STABLE .. master.
>
> Thanks, I was able to reproduce the issue using the steps provided.
> The problem occurs because: When dropping a subscription, it takes an
> AccessExclusiveLock on the pg_subscription system tables to prevent
> the launcher from restarting the worker. During this process, it also
> attempts to connect to the publisher in order to drop the replication
> slot. As we are connecting to a newly created database, it may not yet
> have initialized its catalog caches. As part of the backend startup,
> it attempts to build the cache hierarchy via:
> RelationCacheInitializePhase3 → InitCatalogCachePhase2 →
> InitCatCachePhase2 This cache initialization requires acquiring a
> shared lock on pg_subscription, since it is one of the syscache-backed
> catalog tables. But that shared lock is blocked by the
> AccessExclusiveLock already held by the dropping process. As a result,
> the new backend hangs waiting for the lock, and the original DROP
> SUBSCRIPTION process cannot proceed, leading to a self-blocking
> scenario.
>
> In this specific case, no replication slot was created during
> subscription creation as the connect option was specified as false.
> Therefore, I believe the system should skip connecting to the
> publisher when dropping the subscription. I've attached a patch that
> addresses this behavior. Thoughts?
I think this fix looks correct, while testing I realize that now we
need an additional step before we can enable the subscription, so I
think we should put that additional step in the error hint, as
attached, this top up patch can be applied on top of your patch.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] additinal_errhint.patch (11.7K, ../../CAFiTN-s=iHQ+4u=6qOeNvGrrJ4J7ysfJbejsDx7D4Kpdo1xttA@mail.gmail.com/2-additinal_errhint.patch)
download | inline diff:
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 1d5d3b581f4..0dd952cd569 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -827,7 +827,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
else
ereport(WARNING,
(errmsg("subscription was created, but is not connected"),
- errhint("To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.")));
+ errhint("To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.")));
table_close(rel, RowExclusiveLock);
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index fc42d418bf1..8e4816400ff 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -50,7 +50,7 @@ CREATE PUBLICATION addr_pub_schema FOR TABLES IN SCHEMA addr_nsp;
RESET client_min_messages;
CREATE SUBSCRIPTION regress_addr_sub CONNECTION '' PUBLICATION bar WITH (connect = false, slot_name = NONE);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
CREATE STATISTICS addr_nsp.gentable_stat ON a, b FROM addr_nsp.gentable;
-- test some error cases
SELECT pg_get_object_address('stone', '{}', '{}');
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 180be3c2bde..b01ce15dfaf 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -31,7 +31,7 @@ ERROR: publication name "foo" used more than once
-- ok
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
COMMENT ON SUBSCRIPTION regress_testsub IS 'test subscription';
SELECT obj_description(s.oid, 'pg_subscription') FROM pg_subscription s;
obj_description
@@ -102,7 +102,7 @@ ERROR: subscription with slot_name = NONE must also set enabled = false
-- ok - with slot_name = NONE
CREATE SUBSCRIPTION regress_testsub3 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
-- fail
ALTER SUBSCRIPTION regress_testsub3 ENABLE;
ERROR: cannot enable subscription that does not have a slot name
@@ -114,7 +114,7 @@ ERROR: unrecognized origin value: "foo"
-- now it works
CREATE SUBSCRIPTION regress_testsub4 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (slot_name = NONE, connect = false, origin = none);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
\dRs+ regress_testsub4
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Retain dead tuples | Synchronous commit | Conninfo | Skip LSN
@@ -253,7 +253,7 @@ ERROR: binary requires a Boolean value
-- now it works
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, binary = true);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
\dRs+
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Retain dead tuples | Synchronous commit | Conninfo | Skip LSN
@@ -277,7 +277,7 @@ ERROR: streaming requires a Boolean value or "parallel"
-- now it works
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, streaming = true);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
\dRs+
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Retain dead tuples | Synchronous commit | Conninfo | Skip LSN
@@ -342,7 +342,7 @@ DROP SUBSCRIPTION regress_testsub;
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION mypub
WITH (connect = false, create_slot = false, copy_data = false);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'regress_testsub');
ALTER SUBSCRIPTION regress_testsub ENABLE;
-- fail - ALTER SUBSCRIPTION with refresh is not allowed in a transaction
@@ -370,7 +370,7 @@ ERROR: two_phase requires a Boolean value
-- now it works
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, two_phase = true);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
\dRs+
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Retain dead tuples | Synchronous commit | Conninfo | Skip LSN
@@ -392,7 +392,7 @@ DROP SUBSCRIPTION regress_testsub;
-- two_phase and streaming are compatible.
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, streaming = true, two_phase = true);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
\dRs+
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Retain dead tuples | Synchronous commit | Conninfo | Skip LSN
@@ -408,7 +408,7 @@ ERROR: disable_on_error requires a Boolean value
-- now it works
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, disable_on_error = false);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
\dRs+
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Retain dead tuples | Synchronous commit | Conninfo | Skip LSN
@@ -432,7 +432,7 @@ ERROR: retain_dead_tuples requires a Boolean value
-- ok
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, retain_dead_tuples = false);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
\dRs+
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Retain dead tuples | Synchronous commit | Conninfo | Skip LSN
@@ -467,7 +467,7 @@ GRANT CREATE ON DATABASE REGRESSION TO regress_subscription_user3;
SET SESSION AUTHORIZATION regress_subscription_user3;
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist password=regress_fakepassword' PUBLICATION testpub WITH (connect = false);
WARNING: subscription was created, but is not connected
-HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+HINT: To initiate replication, you must manually create the replication slot, set slot name, enable the subscription, and refresh the subscription.
ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'regress_testsub');
-- we cannot give the subscription away to some random user
ALTER SUBSCRIPTION regress_testsub OWNER TO regress_subscription_user;
^ permalink raw reply [nested|flat] 37+ messages in thread
* RE: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-07-31 09:03 Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 2 replies; 37+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2025-07-31 09:03 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; vignesh C <vignesh21@gmail.com>; +Cc: exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Dear Vignesh, Dilip,
I found another corner case:
```
postgres=# CREATE SUBSCRIPTION sub CONNECTION 'dbname=db1 user=postgres port=5431' PUBLICATION pub1 WITH (connect=false, slot_name=sub);
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
CREATE SUBSCRIPTION
postgres=# DROP SUBSCRIPTION sub ;
... (won't return)
```
Because still can explicitly specify the slot_name while creating the subscription.
Another pattern is to run ALTER SUBSCRIPTION SET (slot_name) command after the
CREATE SUBSCRIPTION WITH (connect=false);.
Should we fix the case? If so, how?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-07-31 11:44 Dilip Kumar <dilipbalaut@gmail.com>
parent: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
1 sibling, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-07-31 11:44 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; +Cc: vignesh C <vignesh21@gmail.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Jul 31, 2025 at 2:34 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Vignesh, Dilip,
>
> I found another corner case:
>
> ```
> postgres=# CREATE SUBSCRIPTION sub CONNECTION 'dbname=db1 user=postgres port=5431' PUBLICATION pub1 WITH (connect=false, slot_name=sub);
> WARNING: subscription was created, but is not connected
> HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
> CREATE SUBSCRIPTION
> postgres=# DROP SUBSCRIPTION sub ;
> ... (won't return)
> ```
>
> Because still can explicitly specify the slot_name while creating the subscription.
> Another pattern is to run ALTER SUBSCRIPTION SET (slot_name) command after the
> CREATE SUBSCRIPTION WITH (connect=false);.
>
> Should we fix the case? If so, how?
IMHO, what we should do is to set a flag in subscription that whether
the connect is true or not, and drop subscription should not try to
drop the slot if the connect is false, thoughts?
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-07-31 11:59 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 0 replies; 37+ messages in thread
From: Dilip Kumar @ 2025-07-31 11:59 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; +Cc: vignesh C <vignesh21@gmail.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Jul 31, 2025 at 5:14 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Jul 31, 2025 at 2:34 PM Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Vignesh, Dilip,
> >
> > I found another corner case:
> >
> > ```
> > postgres=# CREATE SUBSCRIPTION sub CONNECTION 'dbname=db1 user=postgres port=5431' PUBLICATION pub1 WITH (connect=false, slot_name=sub);
> > WARNING: subscription was created, but is not connected
> > HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
> > CREATE SUBSCRIPTION
> > postgres=# DROP SUBSCRIPTION sub ;
> > ... (won't return)
> > ```
> >
> > Because still can explicitly specify the slot_name while creating the subscription.
> > Another pattern is to run ALTER SUBSCRIPTION SET (slot_name) command after the
> > CREATE SUBSCRIPTION WITH (connect=false);.
> >
> > Should we fix the case? If so, how?
>
> IMHO, what we should do is to set a flag in subscription that whether
> the connect is true or not, and drop subscription should not try to
> drop the slot if the connect is false, thoughts?
Maybe not, since this exists in previous branches as well and we
cannot alter the system catalog in back branches. Let me put more
thoughts on this.
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-04 03:37 vignesh C <vignesh21@gmail.com>
parent: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
1 sibling, 1 reply; 37+ messages in thread
From: vignesh C @ 2025-08-04 03:37 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, 31 Jul 2025 at 14:34, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Vignesh, Dilip,
>
> I found another corner case:
>
> ```
> postgres=# CREATE SUBSCRIPTION sub CONNECTION 'dbname=db1 user=postgres port=5431' PUBLICATION pub1 WITH (connect=false, slot_name=sub);
> WARNING: subscription was created, but is not connected
> HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
> CREATE SUBSCRIPTION
> postgres=# DROP SUBSCRIPTION sub ;
> ... (won't return)
> ```
>
> Because still can explicitly specify the slot_name while creating the subscription.
> Another pattern is to run ALTER SUBSCRIPTION SET (slot_name) command after the
> CREATE SUBSCRIPTION WITH (connect=false);.
>
> Should we fix the case? If so, how?
An alternative approach would be to acquire an AccessShareLock on
pg_subscription, and then explicitly lock the target subscription row
using LockSharedObject with AccessExclusiveLock while dropping the
subscription. On the launcher side, before starting a worker, it
should similarly acquire an AccessExclusiveLock on the corresponding
subscription row using LockSharedObject. Once the lock is acquired, it
must revalidate that the subscription still exists, to prevent race
conditions with concurrent drops, before proceeding to start the
worker.
Thoughts?
Regards,
Vignesh
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-04 03:59 Dilip Kumar <dilipbalaut@gmail.com>
parent: vignesh C <vignesh21@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-04 03:59 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 4, 2025 at 9:07 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, 31 Jul 2025 at 14:34, Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Vignesh, Dilip,
> >
> > I found another corner case:
> >
> > ```
> > postgres=# CREATE SUBSCRIPTION sub CONNECTION 'dbname=db1 user=postgres port=5431' PUBLICATION pub1 WITH (connect=false, slot_name=sub);
> > WARNING: subscription was created, but is not connected
> > HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
> > CREATE SUBSCRIPTION
> > postgres=# DROP SUBSCRIPTION sub ;
> > ... (won't return)
> > ```
> >
> > Because still can explicitly specify the slot_name while creating the subscription.
> > Another pattern is to run ALTER SUBSCRIPTION SET (slot_name) command after the
> > CREATE SUBSCRIPTION WITH (connect=false);.
> >
> > Should we fix the case? If so, how?
>
> An alternative approach would be to acquire an AccessShareLock on
> pg_subscription, and then explicitly lock the target subscription row
> using LockSharedObject with AccessExclusiveLock while dropping the
> subscription. On the launcher side, before starting a worker, it
> should similarly acquire an AccessExclusiveLock on the corresponding
> subscription row using LockSharedObject. Once the lock is acquired, it
> must revalidate that the subscription still exists, to prevent race
> conditions with concurrent drops, before proceeding to start the
> worker.
Thanks for the idea, so currently during drop subscription we are
already doing LockSharedObject on the subscription in
AccessExclusiveLock. So now we should try to acquire
AccessExclusiveLock on the launcher side and then revalidate the
object existence, let me try this and post a patch in a while?
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-04 12:28 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-04 12:28 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 4, 2025 at 9:29 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 4, 2025 at 9:07 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Thu, 31 Jul 2025 at 14:34, Hayato Kuroda (Fujitsu)
> > <kuroda.hayato@fujitsu.com> wrote:
> > >
> > > Dear Vignesh, Dilip,
> > >
> > > I found another corner case:
> > >
> > > ```
> > > postgres=# CREATE SUBSCRIPTION sub CONNECTION 'dbname=db1 user=postgres port=5431' PUBLICATION pub1 WITH (connect=false, slot_name=sub);
> > > WARNING: subscription was created, but is not connected
> > > HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
> > > CREATE SUBSCRIPTION
> > > postgres=# DROP SUBSCRIPTION sub ;
> > > ... (won't return)
> > > ```
> > >
> > > Because still can explicitly specify the slot_name while creating the subscription.
> > > Another pattern is to run ALTER SUBSCRIPTION SET (slot_name) command after the
> > > CREATE SUBSCRIPTION WITH (connect=false);.
> > >
> > > Should we fix the case? If so, how?
> >
> > An alternative approach would be to acquire an AccessShareLock on
> > pg_subscription, and then explicitly lock the target subscription row
> > using LockSharedObject with AccessExclusiveLock while dropping the
> > subscription. On the launcher side, before starting a worker, it
> > should similarly acquire an AccessExclusiveLock on the corresponding
> > subscription row using LockSharedObject. Once the lock is acquired, it
> > must revalidate that the subscription still exists, to prevent race
> > conditions with concurrent drops, before proceeding to start the
> > worker.
>
> Thanks for the idea, so currently during drop subscription we are
> already doing LockSharedObject on the subscription in
> AccessExclusiveLock. So now we should try to acquire
> AccessExclusiveLock on the launcher side and then revalidate the
> object existence, let me try this and post a patch in a while?
>
I have worked on this and produced a first version of patch, let's see
what others think about this idea. It would have been better if we
could use SysCache for rechecking the subscription, but since we are
not connected to the database in the launcher we can not use the
SysCache, at least that's what I think.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] fix_drop_subscription_hang.patch (3.0K, ../../CAFiTN-vhc4rEqfXMYkPvaoRbr8_ZFpcBDNV3mUVaOfZfMAdHdQ@mail.gmail.com/2-fix_drop_subscription_hang.patch)
download | inline diff:
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9467f58a23d..9f2f1abb586 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1575,7 +1575,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
* Lock pg_subscription with AccessExclusiveLock to ensure that the
* launcher doesn't restart new worker during dropping the subscription
*/
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 294d0d74d8c..bdce4467553 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -35,6 +35,7 @@
#include "replication/walreceiver.h"
#include "replication/worker_internal.h"
#include "storage/ipc.h"
+#include "storage/lmgr.h"
#include "storage/proc.h"
#include "storage/procarray.h"
#include "tcop/tcopprot.h"
@@ -171,6 +172,43 @@ get_subscription_list(void)
return res;
}
+/*
+ * Check subscription exists or not.
+ */
+static bool
+is_subscription_exist(Oid subid)
+{
+ Relation rel;
+ TableScanDesc scan;
+ HeapTuple tup;
+ bool found = false;
+
+ /* Start a transaction, for detail see comments in get_subscription_list */
+ StartTransactionCommand();
+ (void) GetTransactionSnapshot();
+
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
+ scan = table_beginscan_catalog(rel, 0, NULL);
+
+ while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection)))
+ {
+ Form_pg_subscription subform = (Form_pg_subscription) GETSTRUCT(tup);
+
+ if (subform->oid == subid)
+ {
+ found = true;
+ break;
+ }
+ }
+
+ table_endscan(scan);
+ table_close(rel, AccessShareLock);
+
+ CommitTransactionCommand();
+
+ return found;
+}
+
/*
* Wait for a background worker to start up and attach to the shmem context.
*
@@ -1209,6 +1247,19 @@ ApplyLauncherMain(Datum main_arg)
if (last_start == 0 ||
(elapsed = TimestampDifferenceMilliseconds(last_start, now)) >= wal_retrieve_retry_interval)
{
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, sub->oid, 0,
+ AccessShareLock);
+ if (!is_subscription_exist(sub->oid))
+ {
+ UnlockSharedObject(SubscriptionRelationId, sub->oid, 0,
+ AccessShareLock);
+ continue;
+ }
+
ApplyLauncherSetWorkerStartTime(sub->oid, now);
if (!logicalrep_worker_launch(WORKERTYPE_APPLY,
sub->dbid, sub->oid, sub->name,
@@ -1225,6 +1276,8 @@ ApplyLauncherMain(Datum main_arg)
wait_time = Min(wait_time,
wal_retrieve_retry_interval);
}
+ UnlockSharedObject(SubscriptionRelationId, sub->oid, 0,
+ AccessShareLock);
}
else
{
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-04 12:50 Álvaro Herrera <alvherre@kurilemu.de>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Álvaro Herrera @ 2025-08-04 12:50 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On 2025-Aug-04, Dilip Kumar wrote:
> I have worked on this and produced a first version of patch, let's see
> what others think about this idea. It would have been better if we
> could use SysCache for rechecking the subscription, but since we are
> not connected to the database in the launcher we can not use the
> SysCache, at least that's what I think.
I think it's reasonable to recheck after locking. There's a comment in
DropSubscription that says we get AEL, which is no longer true. In
is_subscription_exists() you should use the index on OID instead of
seqscanning the catalog without a scankey; also I think the name ought
to be "does" rather than "is". I think it's really odd that that
function opens and closes a transaction; sounds to me that something
like that really belongs in the caller (frankly the same is true with
the other function that your comment references). Why isn't
systable_beginscan being used to scan the catalog?
I think with this coding, the resource owner for this new lock is NULL.
Is this really a good approach? Maybe there should be a resowner here.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Always assume the user will do much worse than the stupidest thing
you can imagine." (Julien PUYDT)
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-04 13:38 Dilip Kumar <dilipbalaut@gmail.com>
parent: Álvaro Herrera <alvherre@kurilemu.de>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-04 13:38 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 4, 2025 at 6:20 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> On 2025-Aug-04, Dilip Kumar wrote:
>
> > I have worked on this and produced a first version of patch, let's see
> > what others think about this idea. It would have been better if we
> > could use SysCache for rechecking the subscription, but since we are
> > not connected to the database in the launcher we can not use the
> > SysCache, at least that's what I think.
>
> I think it's reasonable to recheck after locking. There's a comment in
> DropSubscription that says we get AEL, which is no longer true.
Right, will remove that.
In
> is_subscription_exists() you should use the index on OID instead of
> seqscanning the catalog without a scankey;
I thought since launcher is not connected to the database we will not
be able to open the index relation. Otherwise we may just call
SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); Maybe this
is not connected because it was not required so far and we can just
connect it to template1 ?
also I think the name ought
> to be "does" rather than "is".
Okay
I think it's really odd that that
> function opens and closes a transaction; sounds to me that something
> like that really belongs in the caller (frankly the same is true with
> the other function that your comment references). Why isn't
> systable_beginscan being used to scan the catalog?
You mean for this function or for get_subscription_list() as well,
yeah logically systable_beginscan() sounds better.
> I think with this coding, the resource owner for this new lock is NULL.
> Is this really a good approach? Maybe there should be a resowner here.
As you suggested we should move the transaction to the caller and
start it before LockSharedObject() so that we will acquire the lock
under the TopTransactionResourceOwner ?
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-05 06:21 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-05 06:21 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 4, 2025 at 7:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 4, 2025 at 6:20 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> >
> > On 2025-Aug-04, Dilip Kumar wrote:
> >
> > > I have worked on this and produced a first version of patch, let's see
> > > what others think about this idea. It would have been better if we
> > > could use SysCache for rechecking the subscription, but since we are
> > > not connected to the database in the launcher we can not use the
> > > SysCache, at least that's what I think.
> >
> > I think it's reasonable to recheck after locking. There's a comment in
> > DropSubscription that says we get AEL, which is no longer true.
>
> Right, will remove that.
>
> In
> > is_subscription_exists() you should use the index on OID instead of
> > seqscanning the catalog without a scankey;
>
> I thought since launcher is not connected to the database we will not
> be able to open the index relation. Otherwise we may just call
> SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); Maybe this
> is not connected because it was not required so far and we can just
> connect it to template1 ?
>
>
> also I think the name ought
> > to be "does" rather than "is".
>
> Okay
>
> I think it's really odd that that
> > function opens and closes a transaction; sounds to me that something
> > like that really belongs in the caller (frankly the same is true with
> > the other function that your comment references). Why isn't
> > systable_beginscan being used to scan the catalog?
>
> You mean for this function or for get_subscription_list() as well,
> yeah logically systable_beginscan() sounds better.
>
> > I think with this coding, the resource owner for this new lock is NULL.
> > Is this really a good approach? Maybe there should be a resowner here.
>
> As you suggested we should move the transaction to the caller and
> start it before LockSharedObject() so that we will acquire the lock
> under the TopTransactionResourceOwner ?
Here is revised version based on what I proposed here
- I have removed the comment in DropSubscription where we acquire the
lock, as mentioning the ASL is not interesting anymore, instead I am
explaining in launcher why we are acquiring shared object lock.
- Connected launcher to "postgres" database so that we can do syscache lookup
- got rid of "is_subscription_exists" as we are directly validating
using syscache lookup
- Didn't do anything with existing function i.e. get_subscription_list
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v1-0001-Fix-drop-subcription-deadlock-with-create-databas.patch (4.0K, ../../CAFiTN-uWQgNa+4F8D=TEpeib-ouqRiMZTybwGd3wjRNLik5fdQ@mail.gmail.com/2-v1-0001-Fix-drop-subcription-deadlock-with-create-databas.patch)
download | inline diff:
From ca245e5934f2225fa3e71b40792bab2a2d41620b Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Tue, 5 Aug 2025 05:40:12 +0000
Subject: [PATCH v1] Fix drop subcription deadlock with create database
The DROP SUBSCRIPTION command previously acquired an AccessExclusiveLock
on the pg_subscription catalog to prevent the launcher from starting new
workers. This created a deadlock because new database connections require
an AccessShareLock on the same catalog to build their cache.
This commit resolves the deadlock by downgrading the lock on the pg_subscription
catalog to AccessShareLock during drop subscriber. To prevent the launcher from
starting a new worker for the subscription being dropped, we acquired a shared
object lock on the subscription, which was already held by DROP SUBSCRIPTION in
exclusive mode.
---
src/backend/commands/subscriptioncmds.c | 6 +---
src/backend/replication/logical/launcher.c | 32 ++++++++++++++++++----
2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9467f58a23d..05478e53530 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1571,11 +1571,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 294d0d74d8c..f3c48e9b4f0 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -35,6 +35,7 @@
#include "replication/walreceiver.h"
#include "replication/worker_internal.h"
#include "storage/ipc.h"
+#include "storage/lmgr.h"
#include "storage/proc.h"
#include "storage/procarray.h"
#include "tcop/tcopprot.h"
@@ -42,6 +43,7 @@
#include "utils/memutils.h"
#include "utils/pg_lsn.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
/* max sleep time between cycles (3min) */
#define DEFAULT_NAPTIME_PER_CYCLE 180000L
@@ -1147,11 +1149,9 @@ ApplyLauncherMain(Datum main_arg)
pqsignal(SIGTERM, die);
BackgroundWorkerUnblockSignals();
- /*
- * Establish connection to nailed catalogs (we only ever access
- * pg_subscription).
- */
- BackgroundWorkerInitializeConnection(NULL, NULL, 0);
+
+ /* Connect to postgres, as we need to look up the syscache. */
+ BackgroundWorkerInitializeConnection("postgres", NULL, 0);
/* Enter main loop */
for (;;)
@@ -1209,6 +1209,25 @@ ApplyLauncherMain(Datum main_arg)
if (last_start == 0 ||
(elapsed = TimestampDifferenceMilliseconds(last_start, now)) >= wal_retrieve_retry_interval)
{
+ HeapTuple tup;
+
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ StartTransactionCommand();
+ LockSharedObject(SubscriptionRelationId, sub->oid, 0,
+ AccessShareLock);
+ tup = SearchSysCache1(SUBSCRIPTIONOID,
+ ObjectIdGetDatum(sub->oid));
+ if (!HeapTupleIsValid(tup))
+ {
+ UnlockSharedObject(SubscriptionRelationId, sub->oid, 0,
+ AccessShareLock);
+ CommitTransactionCommand();
+ continue;
+ }
+
ApplyLauncherSetWorkerStartTime(sub->oid, now);
if (!logicalrep_worker_launch(WORKERTYPE_APPLY,
sub->dbid, sub->oid, sub->name,
@@ -1225,6 +1244,9 @@ ApplyLauncherMain(Datum main_arg)
wait_time = Min(wait_time,
wal_retrieve_retry_interval);
}
+ UnlockSharedObject(SubscriptionRelationId, sub->oid, 0,
+ AccessShareLock);
+ CommitTransactionCommand();
}
else
{
--
2.50.1.565.gc32cd1483b-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-06 04:58 Amit Kapila <amit.kapila16@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Amit Kapila @ 2025-08-06 04:58 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Tue, Aug 5, 2025 at 11:52 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 4, 2025 at 7:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Mon, Aug 4, 2025 at 6:20 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> > >
> > > On 2025-Aug-04, Dilip Kumar wrote:
> > >
> > > > I have worked on this and produced a first version of patch, let's see
> > > > what others think about this idea. It would have been better if we
> > > > could use SysCache for rechecking the subscription, but since we are
> > > > not connected to the database in the launcher we can not use the
> > > > SysCache, at least that's what I think.
> > >
> > > I think it's reasonable to recheck after locking. There's a comment in
> > > DropSubscription that says we get AEL, which is no longer true.
> >
> > Right, will remove that.
> >
> > In
> > > is_subscription_exists() you should use the index on OID instead of
> > > seqscanning the catalog without a scankey;
> >
> > I thought since launcher is not connected to the database we will not
> > be able to open the index relation. Otherwise we may just call
> > SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); Maybe this
> > is not connected because it was not required so far and we can just
> > connect it to template1 ?
> >
> >
> > also I think the name ought
> > > to be "does" rather than "is".
> >
> > Okay
> >
> > I think it's really odd that that
> > > function opens and closes a transaction; sounds to me that something
> > > like that really belongs in the caller (frankly the same is true with
> > > the other function that your comment references). Why isn't
> > > systable_beginscan being used to scan the catalog?
> >
> > You mean for this function or for get_subscription_list() as well,
> > yeah logically systable_beginscan() sounds better.
> >
> > > I think with this coding, the resource owner for this new lock is NULL.
> > > Is this really a good approach? Maybe there should be a resowner here.
> >
> > As you suggested we should move the transaction to the caller and
> > start it before LockSharedObject() so that we will acquire the lock
> > under the TopTransactionResourceOwner ?
>
> Here is revised version based on what I proposed here
>
> - I have removed the comment in DropSubscription where we acquire the
> lock, as mentioning the ASL is not interesting anymore, instead I am
> explaining in launcher why we are acquiring shared object lock.
> - Connected launcher to "postgres" database so that we can do syscache lookup
>
Won't that add a new restriction that one can't drop postgres database
after this?
BTW, isn't it better to acquire the share_lock on subscription during
worker initialization (InitializeLogRepWorker()) where we already
checking whether the subscription is dropped by that time? I think if
we don't acquire the lock on subscription during launcher or during
InitializeLogRepWorker(), there is a risk that DropSubscription would
have reached the place where it would have tried to stop the workers
and launcher will launch the worker after that point. Then there is a
possibility of dangling worker because the GetSubscription() can still
return valid value as the transaction in which we are dropping a
subscription could still be in-progress.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-06 05:37 Dilip Kumar <dilipbalaut@gmail.com>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-06 05:37 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Aug 6, 2025 at 10:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> Won't that add a new restriction that one can't drop postgres database
> after this?
That's correct, so maybe this is not acceptable IMHO.
> BTW, isn't it better to acquire the share_lock on subscription during
> worker initialization (InitializeLogRepWorker()) where we already
> checking whether the subscription is dropped by that time? I think if
> we don't acquire the lock on subscription during launcher or during
> InitializeLogRepWorker(), there is a risk that DropSubscription would
> have reached the place where it would have tried to stop the workers
> and launcher will launch the worker after that point. Then there is a
> possibility of dangling worker because the GetSubscription() can still
> return valid value as the transaction in which we are dropping a
> subscription could still be in-progress.
Here is my thought on these 2 approaches, so if we lock in launcher
and check there we avoid launching the worker altogether but for that
we will have to revalidate the subscription using sequence scan on
pg_subcription as we can not open additional indexes as we are not
connected to any database OTOH if we acquire lock in
InitializeLogRepWorker() we don't need to do any additional scan but
we will have to launch the worker and after that if subscription
recheck shows its dropped we will exit the worker.
I think either way it's fine because this is not a very common
performance path, I prefer what you proposed as we will have to add
less code in this case, because we are already checking the
subscription and just need to acquire the shared object lock in
InitializeLogRepWorker(), lets see what other thinks, meanwhile I will
modify the code with this approach as well.
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-06 06:08 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-06 06:08 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Aug 6, 2025 at 11:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Aug 6, 2025 at 10:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > Won't that add a new restriction that one can't drop postgres database
> > after this?
>
> That's correct, so maybe this is not acceptable IMHO.
>
> > BTW, isn't it better to acquire the share_lock on subscription during
> > worker initialization (InitializeLogRepWorker()) where we already
> > checking whether the subscription is dropped by that time? I think if
> > we don't acquire the lock on subscription during launcher or during
> > InitializeLogRepWorker(), there is a risk that DropSubscription would
> > have reached the place where it would have tried to stop the workers
> > and launcher will launch the worker after that point. Then there is a
> > possibility of dangling worker because the GetSubscription() can still
> > return valid value as the transaction in which we are dropping a
> > subscription could still be in-progress.
>
> Here is my thought on these 2 approaches, so if we lock in launcher
> and check there we avoid launching the worker altogether but for that
> we will have to revalidate the subscription using sequence scan on
> pg_subcription as we can not open additional indexes as we are not
> connected to any database OTOH if we acquire lock in
> InitializeLogRepWorker() we don't need to do any additional scan but
> we will have to launch the worker and after that if subscription
> recheck shows its dropped we will exit the worker.
>
> I think either way it's fine because this is not a very common
> performance path, I prefer what you proposed as we will have to add
> less code in this case, because we are already checking the
> subscription and just need to acquire the shared object lock in
> InitializeLogRepWorker(), lets see what other thinks, meanwhile I will
> modify the code with this approach as well.
Here is a patch with a different approach. IMHO with this approach
the code change is much simpler.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v2-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-.patch (3.2K, ../../CAFiTN-vY08aLf-HOBNsFfFmSO0=wry-x_ayRe4LTQRgwJXke0g@mail.gmail.com/2-v2-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-.patch)
download | inline diff:
From ad4ad1dac5cf443dee8b069c16b6e29ae31a8a6e Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 6 Aug 2025 05:45:14 +0000
Subject: [PATCH v2] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 12 ++++++++++++
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9467f58a23d..05478e53530 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1571,11 +1571,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 1bff6c92dda..136982ba211 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4613,6 +4613,12 @@ InitializeLogRepWorker(void)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -4624,9 +4630,15 @@ InitializeLogRepWorker(void)
if (am_leader_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
+ UnlockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid,
+ 0, AccessShareLock);
+
proc_exit(0);
}
+ UnlockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
+
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
--
2.50.1.565.gc32cd1483b-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-07 13:48 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-07 13:48 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Aug 6, 2025 at 11:38 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Aug 6, 2025 at 11:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Wed, Aug 6, 2025 at 10:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > Won't that add a new restriction that one can't drop postgres database
> > > after this?
> >
> > That's correct, so maybe this is not acceptable IMHO.
> >
> > > BTW, isn't it better to acquire the share_lock on subscription during
> > > worker initialization (InitializeLogRepWorker()) where we already
> > > checking whether the subscription is dropped by that time? I think if
> > > we don't acquire the lock on subscription during launcher or during
> > > InitializeLogRepWorker(), there is a risk that DropSubscription would
> > > have reached the place where it would have tried to stop the workers
> > > and launcher will launch the worker after that point. Then there is a
> > > possibility of dangling worker because the GetSubscription() can still
> > > return valid value as the transaction in which we are dropping a
> > > subscription could still be in-progress.
> >
> > Here is my thought on these 2 approaches, so if we lock in launcher
> > and check there we avoid launching the worker altogether but for that
> > we will have to revalidate the subscription using sequence scan on
> > pg_subcription as we can not open additional indexes as we are not
> > connected to any database OTOH if we acquire lock in
> > InitializeLogRepWorker() we don't need to do any additional scan but
> > we will have to launch the worker and after that if subscription
> > recheck shows its dropped we will exit the worker.
> >
> > I think either way it's fine because this is not a very common
> > performance path, I prefer what you proposed as we will have to add
> > less code in this case, because we are already checking the
> > subscription and just need to acquire the shared object lock in
> > InitializeLogRepWorker(), lets see what other thinks, meanwhile I will
> > modify the code with this approach as well.
>
> Here is a patch with a different approach. IMHO with this approach
> the code change is much simpler.
I have slightly modified the patch, basically removing explicitly
unlocking the shared object as that will be taken care by transaction
commit / proc_exit()
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-.patch (3.2K, ../../CAFiTN-t6ixxsZ7qagppaspocFGurd=Xg33y2M=140R==kiTGng@mail.gmail.com/2-v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-.patch)
download | inline diff:
From 4841dcf33fa52b9be27d73b949bff85a8c82172f Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 6 Aug 2025 05:45:14 +0000
Subject: [PATCH v3] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9467f58a23d..05478e53530 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1571,11 +1571,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 1bff6c92dda..31fc6fca312 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4613,6 +4613,12 @@ InitializeLogRepWorker(void)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -4624,9 +4630,18 @@ InitializeLogRepWorker(void)
if (am_leader_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
+ /*
+ * The shared object lock on subid is automatically released by
+ * proc_exit(), so no explicit unlock is necessary here.
+ */
+
proc_exit(0);
}
+ /*
+ * The shared object lock on subid will be released at transaction end.
+ */
+
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
--
2.50.1.565.gc32cd1483b-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-13 05:46 vignesh C <vignesh21@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: vignesh C @ 2025-08-13 05:46 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, 7 Aug 2025 at 19:18, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Aug 6, 2025 at 11:38 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Wed, Aug 6, 2025 at 11:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Wed, Aug 6, 2025 at 10:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > Won't that add a new restriction that one can't drop postgres database
> > > > after this?
> > >
> > > That's correct, so maybe this is not acceptable IMHO.
> > >
> > > > BTW, isn't it better to acquire the share_lock on subscription during
> > > > worker initialization (InitializeLogRepWorker()) where we already
> > > > checking whether the subscription is dropped by that time? I think if
> > > > we don't acquire the lock on subscription during launcher or during
> > > > InitializeLogRepWorker(), there is a risk that DropSubscription would
> > > > have reached the place where it would have tried to stop the workers
> > > > and launcher will launch the worker after that point. Then there is a
> > > > possibility of dangling worker because the GetSubscription() can still
> > > > return valid value as the transaction in which we are dropping a
> > > > subscription could still be in-progress.
> > >
> > > Here is my thought on these 2 approaches, so if we lock in launcher
> > > and check there we avoid launching the worker altogether but for that
> > > we will have to revalidate the subscription using sequence scan on
> > > pg_subcription as we can not open additional indexes as we are not
> > > connected to any database OTOH if we acquire lock in
> > > InitializeLogRepWorker() we don't need to do any additional scan but
> > > we will have to launch the worker and after that if subscription
> > > recheck shows its dropped we will exit the worker.
> > >
> > > I think either way it's fine because this is not a very common
> > > performance path, I prefer what you proposed as we will have to add
> > > less code in this case, because we are already checking the
> > > subscription and just need to acquire the shared object lock in
> > > InitializeLogRepWorker(), lets see what other thinks, meanwhile I will
> > > modify the code with this approach as well.
> >
> > Here is a patch with a different approach. IMHO with this approach
> > the code change is much simpler.
>
> I have slightly modified the patch, basically removing explicitly
> unlocking the shared object as that will be taken care by transaction
> commit / proc_exit()
Thanks for the updated patch. This change is required for the back
branches too. Currently it is not applying for PG16 and below
branches.
git am v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-.patch
Applying: Fix DROP SUBSCRIPTION deadlock with new database creation
error: patch failed: src/backend/commands/subscriptioncmds.c:1571
error: src/backend/commands/subscriptioncmds.c: patch does not apply
error: patch failed: src/backend/replication/logical/worker.c:4624
error: src/backend/replication/logical/worker.c: patch does not apply
Patch failed at 0001 Fix DROP SUBSCRIPTION deadlock with new database creation
Regards,
Vignesh
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-13 05:50 Dilip Kumar <dilipbalaut@gmail.com>
parent: vignesh C <vignesh21@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-13 05:50 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Aug 13, 2025 at 11:16 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, 7 Aug 2025 at 19:18, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Wed, Aug 6, 2025 at 11:38 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Wed, Aug 6, 2025 at 11:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > On Wed, Aug 6, 2025 at 10:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > > > Won't that add a new restriction that one can't drop postgres database
> > > > > after this?
> > > >
> > > > That's correct, so maybe this is not acceptable IMHO.
> > > >
> > > > > BTW, isn't it better to acquire the share_lock on subscription during
> > > > > worker initialization (InitializeLogRepWorker()) where we already
> > > > > checking whether the subscription is dropped by that time? I think if
> > > > > we don't acquire the lock on subscription during launcher or during
> > > > > InitializeLogRepWorker(), there is a risk that DropSubscription would
> > > > > have reached the place where it would have tried to stop the workers
> > > > > and launcher will launch the worker after that point. Then there is a
> > > > > possibility of dangling worker because the GetSubscription() can still
> > > > > return valid value as the transaction in which we are dropping a
> > > > > subscription could still be in-progress.
> > > >
> > > > Here is my thought on these 2 approaches, so if we lock in launcher
> > > > and check there we avoid launching the worker altogether but for that
> > > > we will have to revalidate the subscription using sequence scan on
> > > > pg_subcription as we can not open additional indexes as we are not
> > > > connected to any database OTOH if we acquire lock in
> > > > InitializeLogRepWorker() we don't need to do any additional scan but
> > > > we will have to launch the worker and after that if subscription
> > > > recheck shows its dropped we will exit the worker.
> > > >
> > > > I think either way it's fine because this is not a very common
> > > > performance path, I prefer what you proposed as we will have to add
> > > > less code in this case, because we are already checking the
> > > > subscription and just need to acquire the shared object lock in
> > > > InitializeLogRepWorker(), lets see what other thinks, meanwhile I will
> > > > modify the code with this approach as well.
> > >
> > > Here is a patch with a different approach. IMHO with this approach
> > > the code change is much simpler.
> >
> > I have slightly modified the patch, basically removing explicitly
> > unlocking the shared object as that will be taken care by transaction
> > commit / proc_exit()
>
> Thanks for the updated patch. This change is required for the back
> branches too. Currently it is not applying for PG16 and below
> branches.
> git am v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-.patch
> Applying: Fix DROP SUBSCRIPTION deadlock with new database creation
> error: patch failed: src/backend/commands/subscriptioncmds.c:1571
> error: src/backend/commands/subscriptioncmds.c: patch does not apply
> error: patch failed: src/backend/replication/logical/worker.c:4624
> error: src/backend/replication/logical/worker.c: patch does not apply
> Patch failed at 0001 Fix DROP SUBSCRIPTION deadlock with new database creation
Right, I was waiting for consensus for the fix, but anyway I will send
the patches for back branches, thanks.
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-13 09:30 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-13 09:30 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Aug 13, 2025 at 11:20 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Aug 13, 2025 at 11:16 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > Thanks for the updated patch. This change is required for the back
> > branches too. Currently it is not applying for PG16 and below
> > branches.
> > git am v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-.patch
> > Applying: Fix DROP SUBSCRIPTION deadlock with new database creation
> > error: patch failed: src/backend/commands/subscriptioncmds.c:1571
> > error: src/backend/commands/subscriptioncmds.c: patch does not apply
> > error: patch failed: src/backend/replication/logical/worker.c:4624
> > error: src/backend/replication/logical/worker.c: patch does not apply
> > Patch failed at 0001 Fix DROP SUBSCRIPTION deadlock with new database creation
>
> Right, I was waiting for consensus for the fix, but anyway I will send
> the patches for back branches, thanks.
I have prepared back branch patches.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V15.patch (3.3K, ../../CAFiTN-sDQHoc5Yx=+mqupzwWJZ5=3eAFmzfWbGz7uxn424jajQ@mail.gmail.com/2-v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V15.patch)
download | inline diff:
From 404441fac6f62141a5a65ee98bd90924d52558dd Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 07:51:55 +0000
Subject: [PATCH v3] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 334717c0e96..1f0b6926c53 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1340,11 +1340,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
Form_pg_subscription form;
List *rstates;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
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..0c2c6209242 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3638,6 +3638,12 @@ ApplyWorkerMain(Datum main_arg)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -3645,9 +3651,18 @@ ApplyWorkerMain(Datum main_arg)
(errmsg("logical replication apply worker for subscription %u will not "
"start because the subscription was removed during startup",
MyLogicalRepWorker->subid)));
+
+ /*
+ * The shared object lock on subid is automatically released by
+ * proc_exit(), so no explicit unlock is necessary here.
+ */
proc_exit(0);
}
+ /*
+ * The shared object lock on subid will be released at transaction end.
+ */
+
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
--
2.51.0.rc0.205.g4a044479a3-goog
[application/octet-stream] v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v14.patch (3.3K, ../../CAFiTN-sDQHoc5Yx=+mqupzwWJZ5=3eAFmzfWbGz7uxn424jajQ@mail.gmail.com/3-v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v14.patch)
download | inline diff:
From b0b17b0502405b2a86c927535ba9ad4a035cfd40 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 07:51:55 +0000
Subject: [PATCH v3] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index b1a2f3f81a2..45043b60fce 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1069,11 +1069,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
Form_pg_subscription form;
List *rstates;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index d1c7d352363..81d637b603a 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3129,6 +3129,12 @@ ApplyWorkerMain(Datum main_arg)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -3136,9 +3142,18 @@ ApplyWorkerMain(Datum main_arg)
(errmsg("logical replication apply worker for subscription %u will not "
"start because the subscription was removed during startup",
MyLogicalRepWorker->subid)));
+
+ /*
+ * The shared object lock on subid is automatically released by
+ * proc_exit(), so no explicit unlock is necessary here.
+ */
proc_exit(0);
}
+ /*
+ * The shared object lock on subid will be released at transaction end.
+ */
+
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
--
2.51.0.rc0.205.g4a044479a3-goog
[application/octet-stream] v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-Master.patch (3.3K, ../../CAFiTN-sDQHoc5Yx=+mqupzwWJZ5=3eAFmzfWbGz7uxn424jajQ@mail.gmail.com/4-v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-Master.patch)
download | inline diff:
From c4191441c66f0ba9cac7a1fe383c45b1dbf298be Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 08:15:05 +0000
Subject: [PATCH v3] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index faa3650d287..bf7fbb79341 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1802,11 +1802,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 0fdc5de57ba..4688becd34f 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5415,6 +5415,12 @@ InitializeLogRepWorker(void)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -5426,9 +5432,18 @@ InitializeLogRepWorker(void)
if (am_leader_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
+ /*
+ * The shared object lock on subid is automatically released by
+ * proc_exit(), so no explicit unlock is necessary here.
+ */
+
proc_exit(0);
}
+ /*
+ * The shared object lock on subid will be released at transaction end.
+ */
+
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
--
2.51.0.rc0.205.g4a044479a3-goog
[application/octet-stream] v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V13.patch (3.3K, ../../CAFiTN-sDQHoc5Yx=+mqupzwWJZ5=3eAFmzfWbGz7uxn424jajQ@mail.gmail.com/5-v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V13.patch)
download | inline diff:
From 1514e1e759c25e94a70250f0159b609d9e1f5de1 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 07:57:07 +0000
Subject: [PATCH v3] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 40356e97ed2..ad34e9ed528 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -843,11 +843,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
StringInfoData cmd;
Form_pg_subscription form;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index a0e64301a94..2472f2f1c44 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2070,6 +2070,12 @@ ApplyWorkerMain(Datum main_arg)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -2077,9 +2083,18 @@ ApplyWorkerMain(Datum main_arg)
(errmsg("logical replication apply worker for subscription %u will not "
"start because the subscription was removed during startup",
MyLogicalRepWorker->subid)));
+
+ /*
+ * The shared object lock on subid is automatically released by
+ * proc_exit(), so no explicit unlock is necessary here.
+ */
proc_exit(0);
}
+ /*
+ * The shared object lock on subid will be released at transaction end.
+ */
+
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
--
2.51.0.rc0.205.g4a044479a3-goog
[application/octet-stream] v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V16.patch (3.4K, ../../CAFiTN-sDQHoc5Yx=+mqupzwWJZ5=3eAFmzfWbGz7uxn424jajQ@mail.gmail.com/6-v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V16.patch)
download | inline diff:
From e756358fd903e5b3432d0ec5dfbe8b895bd4ecdf Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 07:43:15 +0000
Subject: [PATCH v3] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 17 ++++++++++++++++-
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index e1260fc0e90..77e6462736c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1490,11 +1490,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 4df0a594dc9..df455d65665 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4492,6 +4492,12 @@ InitializeApplyWorker(void)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -4502,10 +4508,19 @@ InitializeApplyWorker(void)
/* Ensure we remove no-longer-useful entry for worker's start time */
if (!am_tablesync_worker() && !am_parallel_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
+
+ /*
+ * The shared object lock on subid is automatically released by
+ * proc_exit(), so no explicit unlock is necessary here.
+ */
proc_exit(0);
}
- MySubscriptionValid = true;
+ /*
+ * The shared object lock on subid will be released at transaction end.
+ */
+
+ MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
if (!MySubscription->enabled)
--
2.51.0.rc0.205.g4a044479a3-goog
[application/octet-stream] v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V17-18.patch (3.3K, ../../CAFiTN-sDQHoc5Yx=+mqupzwWJZ5=3eAFmzfWbGz7uxn424jajQ@mail.gmail.com/7-v3-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V17-18.patch)
download | inline diff:
From 4841dcf33fa52b9be27d73b949bff85a8c82172f Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 6 Aug 2025 05:45:14 +0000
Subject: [PATCH v3] 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 AccessShareLock 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 | 6 +-----
src/backend/replication/logical/worker.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9467f58a23d..05478e53530 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1571,11 +1571,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, MyDatabaseId,
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 1bff6c92dda..31fc6fca312 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4613,6 +4613,12 @@ InitializeLogRepWorker(void)
StartTransactionCommand();
oldctx = MemoryContextSwitchTo(ApplyContext);
+ /*
+ * Lock the subscription to prevent it from being concurrently dropped,
+ * then re-verify its existence.
+ */
+ LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid, 0,
+ AccessShareLock);
MySubscription = GetSubscription(MyLogicalRepWorker->subid, true);
if (!MySubscription)
{
@@ -4624,9 +4630,18 @@ InitializeLogRepWorker(void)
if (am_leader_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
+ /*
+ * The shared object lock on subid is automatically released by
+ * proc_exit(), so no explicit unlock is necessary here.
+ */
+
proc_exit(0);
}
+ /*
+ * The shared object lock on subid will be released at transaction end.
+ */
+
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
--
2.51.0.rc0.205.g4a044479a3-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-14 06:09 Amit Kapila <amit.kapila16@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Amit Kapila @ 2025-08-14 06:09 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Wed, Aug 13, 2025 at 3:00 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> I have prepared back branch patches.
>
@@ -1802,11 +1802,7 @@ DropSubscription(DropSubscriptionStmt *stmt,
bool isTopLevel)
...
- /*
- * Lock pg_subscription with AccessExclusiveLock to ensure that the
- * launcher doesn't restart new worker during dropping the subscription
- */
- rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
We need to acquire RowExclusiveLock here as we are deleting the
subscription row during this operation. See docs [1] and
AlterSubscription() code for reference. Attached is a top-up patch to
fix this for HEAD. Additionally, I have edited a few comments. If you
agree with these changes then please prepare backbranch patches
accordingly.
[1] - https://www.postgresql.org/docs/devel/explicit-locking.html
--
With Regards,
Amit Kapila.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index bf7fbb79341..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1802,7 +1802,13 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- rel = table_open(SubscriptionRelationId, AccessShareLock);
+ /*
+ * 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, RowExclusiveLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 4688becd34f..1c2c9889be7 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5417,7 +5417,8 @@ InitializeLogRepWorker(void)
/*
* Lock the subscription to prevent it from being concurrently dropped,
- * then re-verify its existence.
+ * 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);
@@ -5432,18 +5433,9 @@ InitializeLogRepWorker(void)
if (am_leader_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
- /*
- * The shared object lock on subid is automatically released by
- * proc_exit(), so no explicit unlock is necessary here.
- */
-
proc_exit(0);
}
- /*
- * The shared object lock on subid will be released at transaction end.
- */
-
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
Attachments:
[text/plain] v3_amit.1.patch.txt (1.9K, ../../CAA4eK1+q3hz-67UzxsvaDDvhapquwe+vm+rfY-f3mPptvQto=g@mail.gmail.com/2-v3_amit.1.patch.txt)
download | inline diff:
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index bf7fbb79341..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1802,7 +1802,13 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
List *rstates;
bool must_use_password;
- rel = table_open(SubscriptionRelationId, AccessShareLock);
+ /*
+ * 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, RowExclusiveLock);
tup = SearchSysCache2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 4688becd34f..1c2c9889be7 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5417,7 +5417,8 @@ InitializeLogRepWorker(void)
/*
* Lock the subscription to prevent it from being concurrently dropped,
- * then re-verify its existence.
+ * 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);
@@ -5432,18 +5433,9 @@ InitializeLogRepWorker(void)
if (am_leader_apply_worker())
ApplyLauncherForgetWorkerStartTime(MyLogicalRepWorker->subid);
- /*
- * The shared object lock on subid is automatically released by
- * proc_exit(), so no explicit unlock is necessary here.
- */
-
proc_exit(0);
}
- /*
- * The shared object lock on subid will be released at transaction end.
- */
-
MySubscriptionValid = true;
MemoryContextSwitchTo(oldctx);
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-14 08:35 Dilip Kumar <dilipbalaut@gmail.com>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-14 08:35 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 14, 2025 at 11:39 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Aug 13, 2025 at 3:00 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > I have prepared back branch patches.
> >
>
> @@ -1802,11 +1802,7 @@ DropSubscription(DropSubscriptionStmt *stmt,
> bool isTopLevel)
> ...
> - /*
> - * Lock pg_subscription with AccessExclusiveLock to ensure that the
> - * launcher doesn't restart new worker during dropping the subscription
> - */
> - rel = table_open(SubscriptionRelationId, AccessExclusiveLock);
> + rel = table_open(SubscriptionRelationId, AccessShareLock);
>
> We need to acquire RowExclusiveLock here as we are deleting the
> subscription row during this operation. See docs [1] and
> AlterSubscription() code for reference.
Yeah that's right
Attached is a top-up patch to
> fix this for HEAD. Additionally, I have edited a few comments. If you
> agree with these changes then please prepare backbranch patches
> accordingly.
Yeah this looks fine to me. PFA patches for back branches.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V13.patch (3.1K, ../../CAFiTN-tGARU3U-QKSXpU1VeCDiwJUVi-0S8-M8L6M18WJV+D5Q@mail.gmail.com/2-v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V13.patch)
download | inline diff:
From 8c75db3cfab921d495d7d97009f2cfef00f26d6f Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:19:49 +0000
Subject: [PATCH v4] 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 +++++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 40356e97ed2..28ab74ece93 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -844,10 +844,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
Form_pg_subscription form;
/*
- * 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 a0e64301a94..b41738627ca 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2070,6 +2070,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)
{
--
2.51.0.rc0.215.g125493bb4a-goog
[application/octet-stream] v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-master.patch (3.1K, ../../CAFiTN-tGARU3U-QKSXpU1VeCDiwJUVi-0S8-M8L6M18WJV+D5Q@mail.gmail.com/3-v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-master.patch)
download | inline diff:
From c15a1b7141e29356e7db8a8caf217aa4b073a868 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 08:15:05 +0000
Subject: [PATCH v4] 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 +++++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index faa3650d287..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1803,10 +1803,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 0fdc5de57ba..1c2c9889be7 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5415,6 +5415,13 @@ InitializeLogRepWorker(void)
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)
{
--
2.51.0.rc0.215.g125493bb4a-goog
[application/octet-stream] v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V16.patch (3.1K, ../../CAFiTN-tGARU3U-QKSXpU1VeCDiwJUVi-0S8-M8L6M18WJV+D5Q@mail.gmail.com/4-v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V16.patch)
download | inline diff:
From 87935136c739ac64d390f324085fc82c0ef65caf Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:06:54 +0000
Subject: [PATCH v4] 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 +++++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index e1260fc0e90..aebfaecd9d8 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1491,10 +1491,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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 4df0a594dc9..9b5c641941f 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4492,6 +4492,13 @@ InitializeApplyWorker(void)
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)
{
--
2.51.0.rc0.215.g125493bb4a-goog
[application/octet-stream] v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V14_15.patch (3.1K, ../../CAFiTN-tGARU3U-QKSXpU1VeCDiwJUVi-0S8-M8L6M18WJV+D5Q@mail.gmail.com/5-v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V14_15.patch)
download | inline diff:
From 93c1751e8084aed3f69441f6ca8be50b884deff1 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:12:34 +0000
Subject: [PATCH v4] 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 +++++++
2 files changed, 12 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)
{
--
2.51.0.rc0.215.g125493bb4a-goog
[application/octet-stream] v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V17_18.patch (3.1K, ../../CAFiTN-tGARU3U-QKSXpU1VeCDiwJUVi-0S8-M8L6M18WJV+D5Q@mail.gmail.com/6-v4-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-V17_18.patch)
download | inline diff:
From c6550407ca9afe2edafb6790deb70eb958353baf Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:06:54 +0000
Subject: [PATCH v41] 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 +++++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 4ff246cd943..540f23ee590 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1645,10 +1645,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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 fd11805a44c..a13dab00bd9 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4682,6 +4682,13 @@ InitializeLogRepWorker(void)
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)
{
--
2.51.0.rc0.215.g125493bb4a-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-14 08:48 Amit Kapila <amit.kapila16@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Amit Kapila @ 2025-08-14 08:48 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 14, 2025 at 2:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> Yeah this looks fine to me. PFA patches for back branches.
>
Can we add a test based on the scenario reported in this email?
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-14 11:02 Dilip Kumar <dilipbalaut@gmail.com>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-14 11:02 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 14, 2025 at 2:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Aug 14, 2025 at 2:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > Yeah this looks fine to me. PFA patches for back branches.
> >
>
> Can we add a test based on the scenario reported in this email?
At first, I thought we can just add this test in subscription.sql, but
IMHO we should not do that because after testing DROP SUBSCRIPTION
give error we should eventually DROP the subscription for cleanup by
creating the slot and enabling the subscription (as shown below[1])
but I that realized during regression wal_level is not logical so we
are not allowed to create the logical slot. So maybe we should write
it in 100_bugs.pl? Am I missing something?
[1]
-- ok CREATE SUBSCRIPTION WITH (connect=false),
CREATE DATABASE regression_db;
CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regression_db'
PUBLICATION testpub WITH (connect=false);
-- fail, due to non existing slot
-- should not create deadlock with initilizing caches of new databse
DROP SUBSCRIPTION regress_testsub;
SELECT 'init' FROM
pg_create_logical_replication_slot('regress_testsub', 'pgoutput');
-- ok, create missing slot, enable the subscription and drop again
SELECT 'init' FROM
pg_create_logical_replication_slot('regress_testsub', 'pgoutput');
ALTER SUBSCRIPTION regress_testsub ENABLE;
DROP SUBSCRIPTION regress_testsub;
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-14 11:34 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-14 11:34 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 14, 2025 at 4:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Aug 14, 2025 at 2:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Aug 14, 2025 at 2:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > Yeah this looks fine to me. PFA patches for back branches.
> > >
> >
> > Can we add a test based on the scenario reported in this email?
>
> At first, I thought we can just add this test in subscription.sql, but
> IMHO we should not do that because after testing DROP SUBSCRIPTION
> give error we should eventually DROP the subscription for cleanup by
> creating the slot and enabling the subscription (as shown below[1])
> but I that realized during regression wal_level is not logical so we
> are not allowed to create the logical slot. So maybe we should write
> it in 100_bugs.pl? Am I missing something?
>
Another alternative is for cleanup instead of creating a missing slot
we can just set the slot_name=NONE and then drop. I will move ahead
with this approach.
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-15 06:21 Amit Kapila <amit.kapila16@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Amit Kapila @ 2025-08-15 06:21 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Thu, Aug 14, 2025 at 5:04 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Aug 14, 2025 at 4:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Thu, Aug 14, 2025 at 2:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Thu, Aug 14, 2025 at 2:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > Yeah this looks fine to me. PFA patches for back branches.
> > > >
> > >
> > > Can we add a test based on the scenario reported in this email?
> >
> > At first, I thought we can just add this test in subscription.sql, but
> > IMHO we should not do that because after testing DROP SUBSCRIPTION
> > give error we should eventually DROP the subscription for cleanup by
> > creating the slot and enabling the subscription (as shown below[1])
> > but I that realized during regression wal_level is not logical so we
> > are not allowed to create the logical slot. So maybe we should write
> > it in 100_bugs.pl? Am I missing something?
> >
>
> Another alternative is for cleanup instead of creating a missing slot
> we can just set the slot_name=NONE and then drop. I will move ahead
> with this approach.
>
Yeah, that sounds good to me.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-15 06:28 Dilip Kumar <dilipbalaut@gmail.com>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 2 replies; 37+ messages in thread
From: Dilip Kumar @ 2025-08-15 06:28 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Fri, Aug 15, 2025 at 11:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Aug 14, 2025 at 5:04 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Thu, Aug 14, 2025 at 4:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Thu, Aug 14, 2025 at 2:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Thu, Aug 14, 2025 at 2:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > >
> > > > > Yeah this looks fine to me. PFA patches for back branches.
> > > > >
> > > >
> > > > Can we add a test based on the scenario reported in this email?
> > >
> > > At first, I thought we can just add this test in subscription.sql, but
> > > IMHO we should not do that because after testing DROP SUBSCRIPTION
> > > give error we should eventually DROP the subscription for cleanup by
> > > creating the slot and enabling the subscription (as shown below[1])
> > > but I that realized during regression wal_level is not logical so we
> > > are not allowed to create the logical slot. So maybe we should write
> > > it in 100_bugs.pl? Am I missing something?
> > >
> >
> > Another alternative is for cleanup instead of creating a missing slot
> > we can just set the slot_name=NONE and then drop. I will move ahead
> > with this approach.
> >
>
> Yeah, that sounds good to me.
>
PFA, updated patches for v13 to head, make check-world is passing for
all the branches for v15+ we had upgrade test which also run
regression test so we additionally have to set max_wal_senders=1 for
002_pg_upgrade.pl
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-head.patch (6.5K, ../../CAFiTN-vtaY55GD90SQXc9cNxg9uL1A5r3Lu70JxkeHvDFR44yw@mail.gmail.com/2-v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-head.patch)
download | inline diff:
From 59a06062903b9634becb424c8846e91ef51c920c Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 08:15:05 +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 | 13 +++++++++++++
src/test/regress/sql/subscription.sql | 13 +++++++++++++
5 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index faa3650d287..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1803,10 +1803,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 59b6ae7719a..8ed17aa6b69 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5415,6 +5415,13 @@ InitializeLogRepWorker(void)
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 4b5e895809b..284b2ef07e0 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -225,6 +225,8 @@ $oldnode->init(%old_node_params);
# Override log_statement=all set by Cluster.pm. This avoids large amounts
# of log traffic that slow this test down even more when run under valgrind.
$oldnode->append_conf('postgresql.conf', 'log_statement = none');
+$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
+$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
$oldnode->start;
my $result;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index a98c97f7616..d24ba40f5bf 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -441,6 +441,19 @@ HINT: To initiate replication, you must manually create the replication slot, e
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: subscription was created, but is not connected
+HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+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;
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
-- fail, not enough privileges
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index f0f714fe747..22c7eb4e01a 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -298,6 +298,19 @@ CREATE SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist' PUB
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;
+
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
--
2.51.0.rc1.163.g2494970778-goog
[application/octet-stream] v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v15.patch (6.3K, ../../CAFiTN-vtaY55GD90SQXc9cNxg9uL1A5r3Lu70JxkeHvDFR44yw@mail.gmail.com/3-v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v15.patch)
download | inline diff:
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
[application/octet-stream] v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v13.patch (5.6K, ../../CAFiTN-vtaY55GD90SQXc9cNxg9uL1A5r3Lu70JxkeHvDFR44yw@mail.gmail.com/4-v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v13.patch)
download | inline diff:
From 0c562d2aa728424c397badc2bfd02126ee983c13 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:19:49 +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/test/regress/expected/subscription.out | 13 +++++++++++++
src/test/regress/sql/subscription.sql | 13 +++++++++++++
4 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 40356e97ed2..28ab74ece93 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -844,10 +844,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
Form_pg_subscription form;
/*
- * 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 a0e64301a94..b41738627ca 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2070,6 +2070,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/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 67cb15f4f58..b59e6e1af38 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -173,6 +173,19 @@ DROP SUBSCRIPTION IF EXISTS regress_testsub;
NOTICE: subscription "regress_testsub" does not exist, skipping
DROP SUBSCRIPTION regress_testsub; -- fail
ERROR: subscription "regress_testsub" does not exist
+-- 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 the replication slot "regress_testsub" on publisher
+DETAIL: The error was: 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 f0d38ad19bb..44cc31a113e 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -129,6 +129,19 @@ COMMIT;
DROP SUBSCRIPTION IF EXISTS regress_testsub;
DROP SUBSCRIPTION regress_testsub; -- fail
+-- 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
[application/octet-stream] v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v14.patch (5.6K, ../../CAFiTN-vtaY55GD90SQXc9cNxg9uL1A5r3Lu70JxkeHvDFR44yw@mail.gmail.com/5-v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v14.patch)
download | inline diff:
From 9cd1bd70848341995cd0117432dbaef39576df23 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:12:34 +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/test/regress/expected/subscription.out | 12 ++++++++++++
src/test/regress/sql/subscription.sql | 13 +++++++++++++
4 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index b1a2f3f81a2..c09ddbc7a22 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1070,10 +1070,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 d1c7d352363..93a7714922e 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3129,6 +3129,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/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 85bd97ed8dd..d067c639416 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -271,6 +271,18 @@ ALTER SUBSCRIPTION regress_testsub DISABLE;
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
DROP SUBSCRIPTION regress_testsub;
DROP FUNCTION func;
+-- 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 c8a2d08e721..b117d30c3e1 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -209,6 +209,19 @@ ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
DROP SUBSCRIPTION regress_testsub;
DROP FUNCTION func;
+-- 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
[application/octet-stream] v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v16.patch (6.3K, ../../CAFiTN-vtaY55GD90SQXc9cNxg9uL1A5r3Lu70JxkeHvDFR44yw@mail.gmail.com/6-v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v16.patch)
download | inline diff:
From b861a916ee6df47daa143f5f2aa82cb52414526c Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:06:54 +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 | 13 +++++++++++++
src/test/regress/sql/subscription.sql | 13 +++++++++++++
5 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index e1260fc0e90..aebfaecd9d8 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1491,10 +1491,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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 4df0a594dc9..9b5c641941f 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4492,6 +4492,13 @@ InitializeApplyWorker(void)
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 056538af53c..bf98f7f00b4 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -134,6 +134,8 @@ if ($original_provider eq "i")
$node_params{extra} = \@initdb_params;
$oldnode->init(%node_params);
+$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
+$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
$oldnode->start;
my $result;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index b15eddbff3c..51f53637fb9 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -428,6 +428,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);
+WARNING: subscription was created, but is not connected
+HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+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;
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
-- fail, not enough privileges
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 444e563ff3f..a634f9fd4ac 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -290,6 +290,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;
+
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
--
2.51.0.rc1.163.g2494970778-goog
[application/octet-stream] v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v17.patch (6.3K, ../../CAFiTN-vtaY55GD90SQXc9cNxg9uL1A5r3Lu70JxkeHvDFR44yw@mail.gmail.com/7-v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v17.patch)
download | inline diff:
From a42fe14d289909ef2d09db049a97df4bef2e9366 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:06:54 +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 | 13 +++++++++++++
src/test/regress/sql/subscription.sql | 13 +++++++++++++
5 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9467f58a23d..2dc2cdb7340 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1572,10 +1572,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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 1bff6c92dda..f77adfffc83 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4613,6 +4613,13 @@ InitializeLogRepWorker(void)
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 649be522dc0..c3284042eb7 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -182,6 +182,8 @@ if ($oldnode->pg_version >= 15)
$node_params{extra} = \@initdb_params;
$oldnode->init(%node_params);
++$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
++$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
$oldnode->start;
my $result;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 0f2a25cdc19..8c467ea375f 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -428,6 +428,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);
+WARNING: subscription was created, but is not connected
+HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+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;
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
-- fail, not enough privileges
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 3e5ba4cb8c6..ecfd6bf0299 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -290,6 +290,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;
+
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
--
2.51.0.rc1.163.g2494970778-goog
[application/octet-stream] v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v18.patch (6.5K, ../../CAFiTN-vtaY55GD90SQXc9cNxg9uL1A5r3Lu70JxkeHvDFR44yw@mail.gmail.com/8-v5-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-new-database-v18.patch)
download | inline diff:
From c84c657a3b80c2197c27e78cdebbf27ec04d0570 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Thu, 14 Aug 2025 07:06:54 +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 | 13 +++++++++++++
src/test/regress/sql/subscription.sql | 13 +++++++++++++
5 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 4ff246cd943..540f23ee590 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1645,10 +1645,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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 fd11805a44c..a13dab00bd9 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4682,6 +4682,13 @@ InitializeLogRepWorker(void)
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 4b5e895809b..284b2ef07e0 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -225,6 +225,8 @@ $oldnode->init(%old_node_params);
# Override log_statement=all set by Cluster.pm. This avoids large amounts
# of log traffic that slow this test down even more when run under valgrind.
$oldnode->append_conf('postgresql.conf', 'log_statement = none');
+$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
+$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
$oldnode->start;
my $result;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 1443e1d9292..36fa2c74ef8 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -425,6 +425,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);
+WARNING: subscription was created, but is not connected
+HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and refresh the subscription.
+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;
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
-- fail, not enough privileges
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 007c9e70374..fe49a7672f3 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -287,6 +287,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;
+
-- let's do some tests with pg_create_subscription rather than superuser
SET SESSION AUTHORIZATION regress_subscription_user3;
--
2.51.0.rc1.163.g2494970778-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-17 13:53 vignesh C <vignesh21@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
1 sibling, 1 reply; 37+ messages in thread
From: vignesh C @ 2025-08-17 13:53 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Fri, 15 Aug 2025 at 11:58, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Fri, Aug 15, 2025 at 11:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Aug 14, 2025 at 5:04 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Thu, Aug 14, 2025 at 4:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > On Thu, Aug 14, 2025 at 2:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > > > On Thu, Aug 14, 2025 at 2:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > > >
> > > > > > Yeah this looks fine to me. PFA patches for back branches.
> > > > > >
> > > > >
> > > > > Can we add a test based on the scenario reported in this email?
> > > >
> > > > At first, I thought we can just add this test in subscription.sql, but
> > > > IMHO we should not do that because after testing DROP SUBSCRIPTION
> > > > give error we should eventually DROP the subscription for cleanup by
> > > > creating the slot and enabling the subscription (as shown below[1])
> > > > but I that realized during regression wal_level is not logical so we
> > > > are not allowed to create the logical slot. So maybe we should write
> > > > it in 100_bugs.pl? Am I missing something?
> > > >
> > >
> > > Another alternative is for cleanup instead of creating a missing slot
> > > we can just set the slot_name=NONE and then drop. I will move ahead
> > > with this approach.
> > >
> >
> > Yeah, that sounds good to me.
> >
>
> PFA, updated patches for v13 to head, make check-world is passing for
> all the branches for v15+ we had upgrade test which also run
> regression test so we additionally have to set max_wal_senders=1 for
> 002_pg_upgrade.pl
Do you feel it will be better to append the configurations using
single call like below:
-$oldnode->append_conf('postgresql.conf', 'log_statement = none');
-$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
-$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
+$oldnode->append_conf(
+ 'postgresql.conf',
+ qq{log_statement = 'none'
+ wal_level = 'replica'
+ max_wal_senders = 1});
Regards,
Vignesh
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-18 03:04 Dilip Kumar <dilipbalaut@gmail.com>
parent: vignesh C <vignesh21@gmail.com>
0 siblings, 0 replies; 37+ messages in thread
From: Dilip Kumar @ 2025-08-18 03:04 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Sun, Aug 17, 2025 at 7:23 PM vignesh C <vignesh21@gmail.com> wrote:
>
> > PFA, updated patches for v13 to head, make check-world is passing for
> > all the branches for v15+ we had upgrade test which also run
> > regression test so we additionally have to set max_wal_senders=1 for
> > 002_pg_upgrade.pl
>
> Do you feel it will be better to append the configurations using
> single call like below:
> -$oldnode->append_conf('postgresql.conf', 'log_statement = none');
> -$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
> -$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
> +$oldnode->append_conf(
> + 'postgresql.conf',
> + qq{log_statement = 'none'
> + wal_level = 'replica'
> + max_wal_senders = 1});
Not sure if one way is superior over another or preferred. I see both
sorts of usage patterns throughout our test suit.
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-18 03:34 vignesh C <vignesh21@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
1 sibling, 1 reply; 37+ messages in thread
From: vignesh C @ 2025-08-18 03:34 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Fri, 15 Aug 2025 at 11:58, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Fri, Aug 15, 2025 at 11:52 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Aug 14, 2025 at 5:04 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Thu, Aug 14, 2025 at 4:32 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > On Thu, Aug 14, 2025 at 2:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > > >
> > > > > On Thu, Aug 14, 2025 at 2:05 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > > >
> > > > > > Yeah this looks fine to me. PFA patches for back branches.
> > > > > >
> > > > >
> > > > > Can we add a test based on the scenario reported in this email?
> > > >
> > > > At first, I thought we can just add this test in subscription.sql, but
> > > > IMHO we should not do that because after testing DROP SUBSCRIPTION
> > > > give error we should eventually DROP the subscription for cleanup by
> > > > creating the slot and enabling the subscription (as shown below[1])
> > > > but I that realized during regression wal_level is not logical so we
> > > > are not allowed to create the logical slot. So maybe we should write
> > > > it in 100_bugs.pl? Am I missing something?
> > > >
> > >
> > > Another alternative is for cleanup instead of creating a missing slot
> > > we can just set the slot_name=NONE and then drop. I will move ahead
> > > with this approach.
> > >
> >
> > Yeah, that sounds good to me.
> >
>
> PFA, updated patches for v13 to head, make check-world is passing for
> all the branches for v15+ we had upgrade test which also run
> regression test so we additionally have to set max_wal_senders=1 for
> 002_pg_upgrade.pl
This change will run not just the newly added test, but the entire
regression suite under the new configuration. Should we consider
moving this test elsewhere such as into the 100_bugs:
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 4b5e895809b..284b2ef07e0 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -225,6 +225,8 @@ $oldnode->init(%old_node_params);
# Override log_statement=all set by Cluster.pm. This avoids large amounts
# of log traffic that slow this test down even more when run under valgrind.
$oldnode->append_conf('postgresql.conf', 'log_statement = none');
+$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
+$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
Regards,
Vignesh
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-18 06:06 Dilip Kumar <dilipbalaut@gmail.com>
parent: vignesh C <vignesh21@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-18 06:06 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 18, 2025 at 9:04 AM vignesh C <vignesh21@gmail.com> wrote:
> This change will run not just the newly added test, but the entire
> regression suite under the new configuration. Should we consider
> moving this test elsewhere such as into the 100_bugs:
> diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> index 4b5e895809b..284b2ef07e0 100644
> --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> @@ -225,6 +225,8 @@ $oldnode->init(%old_node_params);
> # Override log_statement=all set by Cluster.pm. This avoids large amounts
> # of log traffic that slow this test down even more when run under valgrind.
> $oldnode->append_conf('postgresql.conf', 'log_statement = none');
> +$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
> +$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
Yeah even I didn't like it to set wal_level to replica only for this
test, it makes sense to write it in 100_bugs.pl, here is a updated
patch for HEAD only, I have also updated the commit message, once we
get agreement on this I will prepare a backbranch patches.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v6-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_head.patch (4.6K, ../../CAFiTN-vqR=CcCMorTXpKySW+8RWN2ES_vVM+cS81GzpLzghn8A@mail.gmail.com/2-v6-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_head.patch)
download | inline diff:
From 8649685576b30a0207fc3cddb6c06e6b9f0c0c48 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 08:15:05 +0000
Subject: [PATCH v6] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 25 ++++++++++++++++++++++++
3 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index faa3650d287..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1803,10 +1803,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 8e343873454..22ad9051db3 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5415,6 +5415,13 @@ InitializeLogRepWorker(void)
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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 5e357701183..2c8f16aecae 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -575,4 +575,29 @@ is($result, 't',
$node_publisher->stop('fast');
$node_subscriber->stop('fast');
+# BUG #18988
+# The BUG involved a deadlock between the DROP SUBSCRIPTION command and the
+# walsender process. This occurred when DROP SUBSCRIPTION attempted to remove
+# a replication slot by connecting to a newly created database whose caches
+# are not yet initialized.
+$node_publisher->start();
+
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE test_db;
+ CREATE SUBSCRIPTION test_sub CONNECTION 'dbname=test_db' PUBLICATION test_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) = $node_publisher->psql('postgres',
+ q{DROP SUBSCRIPTION test_sub}
+);
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "test_sub" on publisher/,
+ "could not drop replication slot: error message");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.51.0.rc1.163.g2494970778-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-18 07:25 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-18 07:25 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 18, 2025 at 11:36 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 18, 2025 at 9:04 AM vignesh C <vignesh21@gmail.com> wrote:
> > This change will run not just the newly added test, but the entire
> > regression suite under the new configuration. Should we consider
> > moving this test elsewhere such as into the 100_bugs:
> > diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > index 4b5e895809b..284b2ef07e0 100644
> > --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > @@ -225,6 +225,8 @@ $oldnode->init(%old_node_params);
> > # Override log_statement=all set by Cluster.pm. This avoids large amounts
> > # of log traffic that slow this test down even more when run under valgrind.
> > $oldnode->append_conf('postgresql.conf', 'log_statement = none');
> > +$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
> > +$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
>
> Yeah even I didn't like it to set wal_level to replica only for this
> test, it makes sense to write it in 100_bugs.pl, here is a updated
> patch for HEAD only, I have also updated the commit message, once we
> get agreement on this I will prepare a backbranch patches.
>
Updated test case, cleanup reported by 1) pgperltidy 2) dropped the
database post test and 3) changed subscription name
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v7-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro.patch (4.7K, ../../CAFiTN-so4YSQ_4KaorRRhQLdGv9cNqOkO4fKJyrd9T=KAO5URA@mail.gmail.com/2-v7-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro.patch)
download | inline diff:
From 43ab5139323229e62d74d9be2fbc0a75881bb226 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 08:15:05 +0000
Subject: [PATCH v7] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 26 ++++++++++++++++++++++++
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index faa3650d287..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1803,10 +1803,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 8e343873454..22ad9051db3 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5415,6 +5415,13 @@ InitializeLogRepWorker(void)
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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 5e357701183..17f60a11bb9 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -575,4 +575,30 @@ is($result, 't',
$node_publisher->stop('fast');
$node_subscriber->stop('fast');
+# BUG #18988
+# The BUG involved a deadlock between the DROP SUBSCRIPTION command and the
+# walsender process. This occurred when DROP SUBSCRIPTION attempted to remove
+# a replication slot by connecting to a newly created database whose caches
+# are not yet initialized.
+$node_publisher->start();
+
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION 'dbname=regress_db' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "regress_sub1" on publisher/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.51.0.rc1.163.g2494970778-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-18 09:26 Amit Kapila <amit.kapila16@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Amit Kapila @ 2025-08-18 09:26 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 18, 2025 at 12:55 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 18, 2025 at 11:36 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Mon, Aug 18, 2025 at 9:04 AM vignesh C <vignesh21@gmail.com> wrote:
> > > This change will run not just the newly added test, but the entire
> > > regression suite under the new configuration. Should we consider
> > > moving this test elsewhere such as into the 100_bugs:
> > > diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > > b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > > index 4b5e895809b..284b2ef07e0 100644
> > > --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > > +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> > > @@ -225,6 +225,8 @@ $oldnode->init(%old_node_params);
> > > # Override log_statement=all set by Cluster.pm. This avoids large amounts
> > > # of log traffic that slow this test down even more when run under valgrind.
> > > $oldnode->append_conf('postgresql.conf', 'log_statement = none');
> > > +$oldnode->append_conf('postgresql.conf', 'wal_level = replica');
> > > +$oldnode->append_conf('postgresql.conf', 'max_wal_senders = 1');
> >
> > Yeah even I didn't like it to set wal_level to replica only for this
> > test, it makes sense to write it in 100_bugs.pl, here is a updated
> > patch for HEAD only, I have also updated the commit message, once we
> > get agreement on this I will prepare a backbranch patches.
> >
>
> Updated test case, cleanup reported by 1) pgperltidy 2) dropped the
> database post test and 3) changed subscription name
>
Looks mostly good. How about slightly changing the comment as in attached?
--
With Regards,
Amit Kapila.
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 17f60a11bb9..bccb73e1fa1 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -576,10 +576,13 @@ $node_publisher->stop('fast');
$node_subscriber->stop('fast');
# BUG #18988
-# The BUG involved a deadlock between the DROP SUBSCRIPTION command and the
-# walsender process. This occurred when DROP SUBSCRIPTION attempted to remove
-# a replication slot by connecting to a newly created database whose caches
-# are not yet initialized.
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
$node_publisher->start();
$node_publisher->safe_psql(
Attachments:
[text/plain] v7_amit_1.patch.txt (1012B, ../../CAA4eK1J+O41tZ5i=wgxWqvvuH2Mpnu4eFnwAc4icSVcD4G0=Bg@mail.gmail.com/2-v7_amit_1.patch.txt)
download | inline diff:
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 17f60a11bb9..bccb73e1fa1 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -576,10 +576,13 @@ $node_publisher->stop('fast');
$node_subscriber->stop('fast');
# BUG #18988
-# The BUG involved a deadlock between the DROP SUBSCRIPTION command and the
-# walsender process. This occurred when DROP SUBSCRIPTION attempted to remove
-# a replication slot by connecting to a newly created database whose caches
-# are not yet initialized.
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
$node_publisher->start();
$node_publisher->safe_psql(
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-18 14:21 Dilip Kumar <dilipbalaut@gmail.com>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-18 14:21 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 18, 2025 at 2:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> Looks mostly good. How about slightly changing the comment as in attached?
Your suggestion LGTM, I am making back branch patches. I observed
another behavior change in the test in v13. The $sdterr is matching
with error details not with the error code, so for v13 i have to
modify the error comparison string as [1] whereas with other versions
it's like [2]. But if we execute this test from psql then I can see
the same error in v13 as well[3]. Not sure why perl is behaving
differently. Maybe there is a way to change this but I could not
figure it out. And another difference I have noticed is that in v13 I
had to give complete publisher_connstr that's maybe it's not the
starting server on the default port. From this I realized that maybe
for other branches where it is working fine with just dbname, we
should also include publisher_connstr as shown [4] just to ensure we
are connecting to the right node, thoughts?
[1]
+like(
+ $stderr,
+ qr/ERROR: replication slot "regress_sub1" does not exist/,
+ "could not drop replication slot: error message");
[2]
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "regress_sub1" on publisher/,
+ "could not drop replication slot: error message");
[3]
postgres[2045257]=# DROP SUBSCRIPTION regress_sub1;
ERROR: XX000: could not drop the replication slot "regress_sub1" on publisher
DETAIL: The error was: ERROR: replication slot "regress_sub1" does not exist
LOCATION: DropSubscription, subscriptioncmds.c:1008
[4]
+$publisher_connstr = $node_publisher->connstr . ' dbname=regress_db';
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr'
PUBLICATION regress_pub WITH (connect=false);
+));
Attaching only v13 and head patches, once we have a way forward I will
send other patches as well.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_head.patch (4.8K, ../../CAFiTN-vUcOne9RKQCEOMm870Bf9pUhjnyroXmwWvvR2yskta5Q@mail.gmail.com/2-v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_head.patch)
download | inline diff:
From c29f6c1b37dfb12c694d2a99e6d7df5fd83ea17d Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 08:15:05 +0000
Subject: [PATCH v8] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 29 ++++++++++++++++++++++++
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index faa3650d287..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1803,10 +1803,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 8e343873454..22ad9051db3 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5415,6 +5415,13 @@ InitializeLogRepWorker(void)
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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 5e357701183..bccb73e1fa1 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -575,4 +575,33 @@ is($result, 't',
$node_publisher->stop('fast');
$node_subscriber->stop('fast');
+# BUG #18988
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
+$node_publisher->start();
+
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION 'dbname=regress_db' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "regress_sub1" on publisher/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.51.0.rc1.163.g2494970778-goog
[application/octet-stream] v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v13.patch (5.0K, ../../CAFiTN-vUcOne9RKQCEOMm870Bf9pUhjnyroXmwWvvR2yskta5Q@mail.gmail.com/3-v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v13.patch)
download | inline diff:
From 398134f16d084ae2ca636f214553c84c5da698db Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Mon, 18 Aug 2025 13:03:52 +0000
Subject: [PATCH v8] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 32 +++++++++++++++++++++++-
3 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 40356e97ed2..28ab74ece93 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -844,10 +844,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
Form_pg_subscription form;
/*
- * 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 a0e64301a94..b41738627ca 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2070,6 +2070,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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 7ebb97bbcfa..af007273115 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -3,7 +3,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 9;
+use Test::More tests => 11;
# Bug #15114
@@ -291,3 +291,33 @@ is( $node_subscriber_d_cols->safe_psql(
$node_publisher_d_cols->stop('fast');
$node_subscriber_d_cols->stop('fast');
+
+# BUG #18988
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
+$node_publisher->start();
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=regress_db';
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: replication slot "regress_sub1" does not exist/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
--
2.51.0.rc1.163.g2494970778-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-18 16:41 vignesh C <vignesh21@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: vignesh C @ 2025-08-18 16:41 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, 18 Aug 2025 at 19:52, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Aug 18, 2025 at 2:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > Looks mostly good. How about slightly changing the comment as in attached?
>
> Your suggestion LGTM, I am making back branch patches. I observed
> another behavior change in the test in v13. The $sdterr is matching
> with error details not with the error code, so for v13 i have to
> modify the error comparison string as [1] whereas with other versions
> it's like [2]. But if we execute this test from psql then I can see
> the same error in v13 as well[3]. Not sure why perl is behaving
> differently.
The error message in PG13 and master branch is different, that is the
reason the test was failing.
The error in master is: could not drop replication slot whereas the
error message in PG13 is: could not drop the replication slot.
The test passes with the below change in PG13:
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -315,7 +315,7 @@ my ($ret, $stdout, $stderr) =
isnt($ret, 0, "replication slot does not exist: exit code not 0");
like(
$stderr,
- qr/ERROR: replication slot "regress_sub1" does not exist/,
+ qr/ERROR: could not drop the replication slot "regress_sub1"
on publisher/,
"could not drop replication slot: error message");
$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
Regards,
Vignesh
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-19 02:54 Dilip Kumar <dilipbalaut@gmail.com>
parent: vignesh C <vignesh21@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Dilip Kumar @ 2025-08-19 02:54 UTC (permalink / raw)
To: vignesh C <vignesh21@gmail.com>; +Cc: Amit Kapila <amit.kapila16@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Mon, Aug 18, 2025 at 10:11 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Mon, 18 Aug 2025 at 19:52, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Mon, Aug 18, 2025 at 2:56 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > Looks mostly good. How about slightly changing the comment as in attached?
> >
> > Your suggestion LGTM, I am making back branch patches. I observed
> > another behavior change in the test in v13. The $sdterr is matching
> > with error details not with the error code, so for v13 i have to
> > modify the error comparison string as [1] whereas with other versions
> > it's like [2]. But if we execute this test from psql then I can see
> > the same error in v13 as well[3]. Not sure why perl is behaving
> > differently.
>
> The error message in PG13 and master branch is different, that is the
> reason the test was failing.
> The error in master is: could not drop replication slot whereas the
> error message in PG13 is: could not drop the replication slot.
> The test passes with the below change in PG13:
> --- a/src/test/subscription/t/100_bugs.pl
> +++ b/src/test/subscription/t/100_bugs.pl
> @@ -315,7 +315,7 @@ my ($ret, $stdout, $stderr) =
> isnt($ret, 0, "replication slot does not exist: exit code not 0");
> like(
> $stderr,
> - qr/ERROR: replication slot "regress_sub1" does not exist/,
> + qr/ERROR: could not drop the replication slot "regress_sub1"
> on publisher/,
> "could not drop replication slot: error message");
>
> $node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
Oh my bad, the error codes were so similar I didn't notice the
difference, I probably would have just done string search instead of
comparing by reading. It's a classic case where a machine's precision
beats a human's eyes :)
PFA, patches for head to v13. I have also changed the connection
string to $node_publisher->connstr . ' dbname=regress_db'; instead of
assuming it will start on the default port.
make check-world is passing on all the branches.
--
Regards,
Dilip Kumar
Google
Attachments:
[application/octet-stream] v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_head.patch (4.9K, ../../CAFiTN-uTfggWqBXSLne+-VcYES9p46AxuVzyb1pzFR6FmNvybw@mail.gmail.com/2-v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_head.patch)
download | inline diff:
From f6bcafdd1587c21ce0d3a04eee624a46f1137b25 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Wed, 13 Aug 2025 08:15:05 +0000
Subject: [PATCH v8] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 30 ++++++++++++++++++++++++
3 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index faa3650d287..4c01d21b2f3 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1803,10 +1803,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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, ObjectIdGetDatum(MyDatabaseId),
CStringGetDatum(stmt->subname));
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 8e343873454..22ad9051db3 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5415,6 +5415,13 @@ InitializeLogRepWorker(void)
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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 5e357701183..50223054918 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -575,4 +575,34 @@ is($result, 't',
$node_publisher->stop('fast');
$node_subscriber->stop('fast');
+# BUG #18988
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
+$node_publisher->start();
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=regress_db';
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "regress_sub1" on publisher/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.51.0.rc1.167.g924127e9c0-goog
[application/octet-stream] v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v14.patch (5.0K, ../../CAFiTN-uTfggWqBXSLne+-VcYES9p46AxuVzyb1pzFR6FmNvybw@mail.gmail.com/3-v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v14.patch)
download | inline diff:
From 493fbbf060154fb3e50744aa58e260d44dd20fd0 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Mon, 18 Aug 2025 12:46:30 +0000
Subject: [PATCH v8] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 32 +++++++++++++++++++++++-
3 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index b1a2f3f81a2..c09ddbc7a22 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1070,10 +1070,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 d1c7d352363..93a7714922e 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3129,6 +3129,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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index cce91891ab9..235227c7727 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -6,7 +6,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 9;
+use Test::More tests => 11;
# Bug #15114
@@ -362,3 +362,33 @@ is( $node_subscriber_d_cols->safe_psql(
$node_publisher_d_cols->stop('fast');
$node_subscriber_d_cols->stop('fast');
+
+# BUG #18988
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
+$node_publisher->start();
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=regress_db';
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "regress_sub1" on publisher/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
--
2.51.0.rc1.167.g924127e9c0-goog
[application/octet-stream] v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v16_17_18.patch (4.9K, ../../CAFiTN-uTfggWqBXSLne+-VcYES9p46AxuVzyb1pzFR6FmNvybw@mail.gmail.com/4-v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v16_17_18.patch)
download | inline diff:
From e0b849ffc5a93e190e89adeefdbad7ae8fa52618 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Mon, 18 Aug 2025 11:32:21 +0000
Subject: [PATCH v8] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 30 ++++++++++++++++++++++++
3 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index e1260fc0e90..aebfaecd9d8 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1491,10 +1491,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
bool must_use_password;
/*
- * 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 4df0a594dc9..9b5c641941f 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4492,6 +4492,13 @@ InitializeApplyWorker(void)
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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 6744998b620..87643b8e620 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -568,4 +568,34 @@ is($result, 't',
$node_publisher->stop('fast');
$node_subscriber->stop('fast');
+# BUG #18988
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
+$node_publisher->start();
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=regress_db';
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "regress_sub1" on publisher/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.51.0.rc1.167.g924127e9c0-goog
[application/octet-stream] v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v13.patch (5.0K, ../../CAFiTN-uTfggWqBXSLne+-VcYES9p46AxuVzyb1pzFR6FmNvybw@mail.gmail.com/5-v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v13.patch)
download | inline diff:
From ddd75735505aed702a59cc47f310fa2c5266180c Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Mon, 18 Aug 2025 13:03:52 +0000
Subject: [PATCH v8] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 32 +++++++++++++++++++++++-
3 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 40356e97ed2..28ab74ece93 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -844,10 +844,12 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
Form_pg_subscription form;
/*
- * 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 a0e64301a94..b41738627ca 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2070,6 +2070,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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 7ebb97bbcfa..4f6a9ff9fa4 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -3,7 +3,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 9;
+use Test::More tests => 11;
# Bug #15114
@@ -291,3 +291,33 @@ is( $node_subscriber_d_cols->safe_psql(
$node_publisher_d_cols->stop('fast');
$node_subscriber_d_cols->stop('fast');
+
+# BUG #18988
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
+$node_publisher->start();
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=regress_db';
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop the replication slot "regress_sub1/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
--
2.51.0.rc1.167.g924127e9c0-goog
[application/octet-stream] v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v15.patch (4.9K, ../../CAFiTN-uTfggWqBXSLne+-VcYES9p46AxuVzyb1pzFR6FmNvybw@mail.gmail.com/6-v8-0001-Fix-DROP-SUBSCRIPTION-deadlock-with-walsender-pro_v15.patch)
download | inline diff:
From 8b266d6f15c9c6db9460a177f2d6382c1124c41a Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Mon, 18 Aug 2025 12:18:29 +0000
Subject: [PATCH v8] Fix DROP SUBSCRIPTION deadlock with walsender process
The DROP SUBSCRIPTION command caused a deadlock with the walsender
process because of a lock conflict on the pg_subscription catalog.
This happened when the command was trying to drop a replication slot
by connecting to a newly created database with uninitialized caches.
The DROP SUBSCRIPTION command would first acquire an AccessExclusiveLock
on pg_subscription to prevent new workers from starting. However, the
connection to the new database needed to acquire an AccessShareLock on
the same catalog during its initialization. This created a deadlock
where DROP SUBSCRIPTION held its lock and was waiting for the new
connection to complete, while the new connection was simultaneously
blocked from starting because it couldn't acquire its required lock.
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/test/subscription/t/100_bugs.pl | 30 ++++++++++++++++++++++++
3 files changed, 42 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/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 28ca4affbb9..061eb2a32c1 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -418,4 +418,34 @@ is($result, qq(2|f
$node_publisher->stop('fast');
$node_subscriber->stop('fast');
+# BUG #18988
+# The bug happened due to a self-deadlock between the DROP SUBSCRIPTION
+# command and the walsender process for accessing pg_subscription. This
+# occurred when DROP SUBSCRIPTION attempted to remove a replication slot by
+# connecting to a newly created database whose caches are not yet
+# initialized.
+#
+# The bug is fixed by reducing the lock-level during DROP SUBSCRIPTION.
+$node_publisher->start();
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=regress_db';
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ CREATE DATABASE regress_db;
+ CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr' PUBLICATION regress_pub WITH (connect=false);
+));
+
+my ($ret, $stdout, $stderr) =
+ $node_publisher->psql('postgres', q{DROP SUBSCRIPTION regress_sub1});
+
+isnt($ret, 0, "replication slot does not exist: exit code not 0");
+like(
+ $stderr,
+ qr/ERROR: could not drop replication slot "regress_sub1" on publisher/,
+ "could not drop replication slot: error message");
+
+$node_publisher->safe_psql('postgres', "DROP DATABASE regress_db");
+
+$node_publisher->stop('fast');
+
done_testing();
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-19 09:37 Amit Kapila <amit.kapila16@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 37+ messages in thread
From: Amit Kapila @ 2025-08-19 09:37 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Tue, Aug 19, 2025 at 8:24 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> PFA, patches for head to v13.
>
Pushed.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 37+ messages in thread
* Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database
@ 2025-08-21 03:46 Dilip Kumar <dilipbalaut@gmail.com>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 0 replies; 37+ messages in thread
From: Dilip Kumar @ 2025-08-21 03:46 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: vignesh C <vignesh21@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; exclusion@gmail.com <exclusion@gmail.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
On Tue, Aug 19, 2025 at 3:07 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Aug 19, 2025 at 8:24 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > PFA, patches for head to v13.
> >
>
> Pushed.
Thanks.
--
Regards,
Dilip Kumar
Google
^ permalink raw reply [nested|flat] 37+ messages in thread
end of thread, other threads:[~2025-08-21 03:46 UTC | newest]
Thread overview: 37+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-07-16 19:00 BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database PG Bug reporting form <noreply@postgresql.org>
2025-07-30 10:54 ` vignesh C <vignesh21@gmail.com>
2025-07-31 05:20 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-07-31 09:03 ` Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
2025-07-31 11:44 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-07-31 11:59 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-04 03:37 ` vignesh C <vignesh21@gmail.com>
2025-08-04 03:59 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-04 12:28 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-04 12:50 ` Álvaro Herrera <alvherre@kurilemu.de>
2025-08-04 13:38 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-05 06:21 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-06 04:58 ` Amit Kapila <amit.kapila16@gmail.com>
2025-08-06 05:37 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-06 06:08 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-07 13:48 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-13 05:46 ` vignesh C <vignesh21@gmail.com>
2025-08-13 05:50 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-13 09:30 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-14 06:09 ` Amit Kapila <amit.kapila16@gmail.com>
2025-08-14 08:35 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-14 08:48 ` Amit Kapila <amit.kapila16@gmail.com>
2025-08-14 11:02 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-14 11:34 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-15 06:21 ` Amit Kapila <amit.kapila16@gmail.com>
2025-08-15 06:28 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-17 13:53 ` vignesh C <vignesh21@gmail.com>
2025-08-18 03:04 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-18 03:34 ` vignesh C <vignesh21@gmail.com>
2025-08-18 06:06 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-18 07:25 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-18 09:26 ` Amit Kapila <amit.kapila16@gmail.com>
2025-08-18 14:21 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-18 16:41 ` vignesh C <vignesh21@gmail.com>
2025-08-19 02:54 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-08-19 09:37 ` Amit Kapila <amit.kapila16@gmail.com>
2025-08-21 03:46 ` Dilip Kumar <dilipbalaut@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox