On Fri, Jul 25, 2025 at 6:01 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.com> wrote: > > Dear Ajin, > > Thanks for patches! While checking, I recalled that the backpatch policy [1]. > We must not modify definitions of opened functions but this does. Can you > define > another function like UpdateSubscriptionRelStateEx or something on the back > branches? > > Another comment: > ``` > @@ -328,9 +328,13 @@ UpdateSubscriptionRelState(Oid subid, Oid relid, char > state, > Datum values[Natts_pg_subscription_rel]; > bool replaces[Natts_pg_subscription_rel]; > > - LockSharedObject(SubscriptionRelationId, subid, 0, AccessShareLock); > - > - rel = table_open(SubscriptionRelRelationId, RowExclusiveLock); > + if (already_locked) > + rel = table_open(SubscriptionRelRelationId, NoLock); > ``` > > Can we assert that RowExclusiveLock for pg_subscription_rel has already been > acquired, by using CheckRelationOidLockedByMe() family? >
Thanks for your review Kuroda-san, I have changed the logic to not use already_locked and instead check if the locks are taken inside UpdateSubscriptionRelState itself. I've tested this and this works fine. If this logic is acceptable to the reviewers I can update the other patches also in a similar way. regards, Ajin Cherian Fujitsu Australia
PG_15-v5-0001-Fix-a-deadlock-during-ALTER-SUBSCRIPTION.-DROP-PU.patch
Description: Binary data
