Date: Fri, 3 Jul 2026 09:03:24 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>,
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: Re-read subscription state after lock in AlterSubscription
Message-ID: <akd63Cr/TB9G+vUD@bdtpg>
References: <akZUpiDa1UfmzYxL@bdtpg>
 <CAFiTN-ugOn-owrZVr5XsvYKfzMkpVokRYqnmEM7bnXkSfbjT0A@mail.gmail.com>
 <TYRPR01MB121568CC10A0920C7A5ADCCC2F5F52@TYRPR01MB12156.jpnprd01.prod.outlook.com>
 <akZlreb754FkCFrv@bdtpg>
 <OS9PR01MB12149E944D7ECABF0C1DDCCB4F5F42@OS9PR01MB12149.jpnprd01.prod.outlook.com>
 <akc4NBxbNKeSvumI@bdtpg>
 <CAFiTN-tBC0kvymhKiq9hXdr-Cdsdezyyh-X5FCtzfBsAORGOaw@mail.gmail.com>
 <akdOLGFblqA3Yvd6@bdtpg>
 <TY4PR01MB17718E4AA1A80B59B1B6FBF2D94F42@TY4PR01MB17718.jpnprd01.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <TY4PR01MB17718E4AA1A80B59B1B6FBF2D94F42@TY4PR01MB17718.jpnprd01.prod.outlook.com>
Archived-At: 
 <https://www.postgresql.org/message-id/akd63Cr/TB9G%2BvUD%40bdtpg>
Precedence: bulk

Hi,

On Fri, Jul 03, 2026 at 08:08:13AM +0000, Zhijie Hou (Fujitsu) wrote:
> On Friday, July 3, 2026 1:53 PM Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
> > 
> > That said, what about also fixing DropSubscription() like in the 0002 attached?
> > (that would also produce those elog() messages in case of concurrent DROP or
> > ALTER).
> 
> For the patch, I'm not sure if we must repeat the checks twice.

Thanks for looking at it!

> Could we
> simply move the original checks to after we take the lock? At least, the
> GetSubscription() call and the password check can be moved there and old codes
> can be deleted.

I'm not sure which checks you refer to. The ones that are keep before the lock
acquisition are because we don't want to lock an object we don't have privileges
on (see remark 3 in [1]).

> BTW, this may not be strictly related, but I think it's not safe to do the
> ownership check before locking the subscription as well. If the subscription is
> concurrently dropped, a "tuple concurrently updated" error can still occur.

That's right, I explained why in remark number 2 in [1]:

"
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.
"

Does that make sense?

[1]: https://postgr.es/m/akZUpiDa1UfmzYxL%40bdtpg

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com