On Tue, Aug 16, 2022 at 8:36 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Aug 12, 2022 at 10:52 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Fri, Aug 12, 2022 at 12:44 PM Ajin Cherian <itsajin@gmail.com> wrote:
> > >
> >
> > ISTM that the "This" in the first sentence in the second paragraph
> > still indicates the cleanup of the origin tracking and table sync
> > slot. How about not having the common comment for both like the patch
> > I've attached? In addition to this change, I moved the code to drop
> > the origin before stopping the streaming so that it can be close to
> > the slot drop. But feel free to revert this change.
> >
>
> After this change, won't the code in AlterSubscription_refresh()
> related to origin drop [1] needs to be updated? Firstly, now even if
> the state is SYNCDONE, we would have already dropped the origin. Then,
> I think the race mentioned in the comments no longer remains true
> because before dropping the origin in tablesync worker, we would have
> taken a conflicting lock on pg_subscription_rel.
Right.
Probably the same is true for DropSubscription()? That is, we can skip
removing the origin if the state is SYNCDONE (READ-state relations are
already filtered by GetSubscriptionRelations()).
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/