Re: DROP SUBSCRIPTION hangs if sub is disabled in the same transaction

On Wed, Sep 13, 2017 at 8:00 PM, Arseny Sher <a(dot)sher(at)postgrespro(dot)ru> wrote:
> Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
>> We can break this in any number of ways:
>>
>> - (your patch) Kill workers right away after ALTER SUBSCRIPTION DISABLE,
>> thus breaking the appearance of transactional DDL somewhat.
>> ...
>> - Have DROP SUBSCRIPTION attempt to kill workers if the subscription is
>> disabled (and possibly, was changed in the same transaction), which
>> would address this scenario very narrowly.
>
> Actually, my patch is closer to the last variant. I proposed to kill the
> workers in DROP SUBSCRIPTION, and only if we are dropping replication
> origin (which is probably always the case, though). I agree that it is
> somewhat narrow and still slightly violates transactionality of DROP
> SUBSCRIPTION, meaning that its changes (stopped workers) are seen before
> the commit.
>
> However, do we care much about that? Is there any chance that users will
> rely on living apply workers after DROP SUBSCRIPTION, but before the
> transaction commit? In which situation this might be useful?
>
> On the other hand, forbidding to execute disable sub and drop sub in one
> transaction makes it impossible e.g. to drop subscription in trigger as
> long as Postgres doesn't have autonomous transactions.
>
>
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>> ISTM the second of those (refuse to drop an in-use subscription) is
>> by far the least surprising behavior. However, I wonder if there aren't
>> race conditions involved here. What if we haven't yet committed a
>> DROP SUBSCRIPTION, and some new worker starts up after we look for
>> workers?
>
> We hold a lock on subscription till the end of transaction, so workers
> won't start.
>
>> If there aren't variants of that that will break all four options,
>> it's not very obvious why not.
>
> I see it this way:
> * We want effect of drop sub invisible till commit, so we can't stop
> workers before commit.
> * Drop of replication origin needs to be executed in one transaction with
> drop sub, it writes to WAL and so must be executed before commit.
> * Apply worker needs RO for its work, it owns origin for the whole
> lifetime.
>
> Something should be given up here. One more idea that was not yet
> mentioned is to abandon attempts to drop subs and ROs simultenously and
> just garbage-collect replication origins periodically, but that doesn't
> sound as an elegant solution.
>
>
> Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> writes:
>
>>> I don't think this is reliable -- what if worker suddenly dies without
>>> accomplishing the job?
>>
>> The apply worker will be launched by the launcher later. If DROP
>> SUBSCRIPTION is issued before the apply worker launches again, DROP
>> SUBSCRIPTION itself can remove the replication origin.
>
> Why launcher would restart the worker if we already destroyed the
> subscription?

Ah, the apply worker will not launch in that case.

> Consider the sequence of actions:
>
> * We check in DROP SUBSCRIPTION that worker alive and don't remove RO.
> * DROP SUBSCRIPTION commits.
> * Worker is killed by some villain before it had the chance to drop RO.
> It might be killed even before drop sub commit, but after the check,
> we are left with orphan RO anyway.

Hmm yes, we could left with orphan the replication origin. It's not a
good solution.

The second option makes subscription management more complex for
users. Whenever user wants to drop subscription in use, they have to
disable it before dropping the subscription, while CREATE SUBSCRIPTION
starts the logical replication without ALTER SUBSCRIPTION ENABLE. I
guess disallowing DROP SUBSCRIPTION in a transaction would rather be
more useful for users because there would not be a lot of users who
want to manage subscriptions transactionally.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center