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

On Thu, Sep 14, 2017 at 12:04 AM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> 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.
>

Sorry, "The second option" meant the second option of options listed by Peter.

Regards,

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