Re: 024_add_drop_pub.pl might fail due to deadlock

On Wed, Jul 16, 2025 at 8:38 AM Ajin Cherian <itsajin@gmail.com> wrote:
>
> Thanks for the test and confirming the fix. Fixed the comments.
>

         * origin. So passing missing_ok = true.
+         *
+         * Also lock SubscriptionRelationId with AccessShareLock to
+         * prevent any deadlocks with the user concurrently performing
+         * refresh on the subscription.
         */
+        LockSharedObject(SubscriptionRelationId, MyLogicalRepWorker->subid,
+                 0, AccessShareLock);

It seems the patch assumes that the above lock is sufficient because
AlterSubscription will take an AcessExclusiveLock on the same
subscription. So, with this proposal, if both of those become
serialized then the other locking order may not matter. Am I correct
or is there any other theory you have in mind?

If that is the theory then I think we are missing cases where the
apply worker and Alter subscription operates on different
subscriptions.

Consider AlterSubscription_refresh() takes first AccessExclusiveLock
on SubscriptionRelRelationId and then ExclusiveLock on
ReplicationOriginRelationId via replorigin_drop_by_name() . The apply
worker first takes ExclusiveLock on ReplicationOriginRelationId via
replorigin_drop_by_name() and then RowExclusiveLock on
SubscriptionRelRelationId via UpdateSubscriptionRelState(). Won't such
a scenario taking conflicting locks in reverse order can lead to
deadlock at least in PG15?

--
With Regards,
Amit Kapila.