Re: 024_add_drop_pub.pl might fail due to deadlock

Ajin Cherian <itsajin@gmail.com>

From: Ajin Cherian <itsajin@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: vignesh C <vignesh21@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Amit Kapila <amit.kapila16@gmail.com>
Date: 2025-07-28T12:06:52Z
Lists: pgsql-hackers

Attachments

On Fri, Jul 25, 2025 at 6:01 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@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

Commits

  1. Fix a deadlock during ALTER SUBSCRIPTION ... DROP PUBLICATION.