Re: BUG #18988: DROP SUBSCRIPTION locks not-yet-accessed database

Álvaro Herrera <alvherre@kurilemu.de>

From: Álvaro Herrera <alvherre@kurilemu.de>
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>
Date: 2025-08-04T12:50:34Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix self-deadlock during DROP SUBSCRIPTION.

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)