From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Subject: Re-read subscription state after lock in AlterSubscription
Date: Thu, 2 Jul 2026 12:08:06 +0000
Message-ID: <akZUpiDa1UfmzYxL@bdtpg> (raw)
Hi hackers,
while playing with the new ALTER SUBSCRIPTION parameter added in a5918fddf10,
I realized that the subscription is not re-read once we acquire the lock in
AlterSubscription().
This pre-existing issue is now more visible after a5918fddf10:
1/ two concurrent ALTER SUBSCRIPTION SET (conflict_log_destination = 'table')
could result in the second session attempting to create an already-existing
conflict log table, producing a confusing "relation already exists" error:
ERROR: relation "pg_conflict_log_24614" already exists
It's confusing because ALTER SUBSCRIPTION SET (conflict_log_destination = 'table')
would not report an error if the conflict table already exists (and no concurrent
ALTER is running).
2/ a concurrent DROP followed by the ALTER would emit a NOTICE about creating the
conflict log table before failing with "referenced subscription was concurrently
dropped". That sounds like a weird messaging:
NOTICE: created conflict log table "pg_conflict.pg_conflict_log_24620" for subscription "mysub"
ERROR: referenced subscription was concurrently dropped
The attached fixes it by:
- Re-reading the subscription tuple after LockSharedObject() and refreshing the
Subscription struct.
- Moving the local variable assignments to after the re-read.
- Re-checking the password_required privilege restriction after the re-read.
Remarks:
1/ not re-checking password_required after the re-read would still produce a
"tuple concurrently updated" error, but re-checking it allows us to display a
better error message.
2/ the ownership check is intentionally not re-done after the lock because
AlterSubscriptionOwner() does not take AccessExclusiveLock on the subscription
object: it only takes RowExclusiveLock on the pg_subscription catalog table.
This means ownership can change regardless of our lock, making a re-check after
lock acquisition pointless. The existing "tuple concurrently updated" error from
CatalogTupleUpdate() already provides a protection if ownership changes
concurrently.
3/ the "privileges" checks are still also done before the lock acquisition because
we don't want to lock an object we don't have privileges on.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.comAttachments:v1-0001-Re-read-subscription-state-after-lock-in-AlterSub.patch (text/x-diff, 3.9K) download
From 72a53d1991e7cd9d4a52f48284ddc974a0a4ae65 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 2 Jul 2026 11:07:04 +0000
Subject: [PATCH v1] Re-read subscription state after lock in AlterSubscription
AlterSubscription() reads the subscription's catalog state via GetSubscription()
before acquiring AccessExclusiveLock on the subscription object. A concurrent
session that commits a DROP or ALTER between the read and the lock acquisition
leaves the other session acting with stale information once it unblocks.
Fix by:
- Re-reading the subscription tuple after LockSharedObject() and refreshing the
Subscription struct.
- Moving the local variable assignments to after the re-read.- Re-checking the password_required privilege restriction after the re-read.
Remarks:
1/ not re-checking password_required after the re-read would still produce a
"tuple concurrently updated" error, but re-checking it allows us to display a
better error message.
2/ the ownership check is intentionally not re-done after the lock because
AlterSubscriptionOwner() does not take AccessExclusiveLock on the subscription
object: it only takes RowExclusiveLock on the pg_subscription catalog table.
This means ownership can change regardless of our lock, making a re-check after
lock acquisition pointless. The existing "tuple concurrently updated" error from
CatalogTupleUpdate() already provides a protection if ownership changes
concurrently.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by:
Discussion:
---
src/backend/commands/subscriptioncmds.c | 41 ++++++++++++++++++++++---
1 file changed, 36 insertions(+), 5 deletions(-)
100.0% src/backend/commands/
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.cindex 4292e7fb8f4..be03b3eb7e1 100644--- a/src/backend/commands/subscriptioncmds.c+++ b/src/backend/commands/subscriptioncmds.c@@ -1695,11 +1695,6 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
*/
sub = GetSubscription(subid, false, orig_conninfo_needed, false);
- retain_dead_tuples = sub->retaindeadtuples;- origin = sub->origin;- max_retention = sub->maxretention;- retention_active = sub->retentionactive;-
/*
* Don't allow non-superuser modification of a subscription with
* password_required=false.
@@ -1713,6 +1708,42 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
/* Lock the subscription so nobody else can do anything with it. */
LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock);
+ /*+ * Re-read the subscription tuple after acquiring the lock. A concurrent+ * DROP or ALTER may have committed before we acquired the lock.+ */+ heap_freetuple(tup);+ tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId),+ CStringGetDatum(stmt->subname));++ if (!HeapTupleIsValid(tup))+ ereport(ERROR,+ (errcode(ERRCODE_UNDEFINED_OBJECT),+ errmsg("subscription \"%s\" does not exist",+ stmt->subname)));++ form = (Form_pg_subscription) GETSTRUCT(tup);++ /* Refresh the subscription. */+ pfree(sub);+ sub = GetSubscription(subid, false, orig_conninfo_needed, false);++ /*+ * Re-check whether a non-superuser is allowed to alter this subscription.+ * A concurrent ALTER may have set password_required=false while we were+ * waiting for the lock.+ */+ if (!sub->passwordrequired && !superuser())+ ereport(ERROR,+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),+ errmsg("password_required=false is superuser-only"),+ errhint("Subscriptions with the password_required option set to false may only be created or modified by the superuser.")));++ retain_dead_tuples = sub->retaindeadtuples;+ origin = sub->origin;+ max_retention = sub->maxretention;+ retention_active = sub->retentionactive;+
/* Form a new tuple. */
memset(values, 0, sizeof(values));
memset(nulls, false, sizeof(nulls));
--
2.34.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Re-read subscription state after lock in AlterSubscription
In-Reply-To: <akZUpiDa1UfmzYxL@bdtpg>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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