Re: DROP SUBSCRIPTION and ROLLBACK

On Wed, Feb 8, 2017 at 12:26 AM, Petr Jelinek
<petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
> On 07/02/17 13:10, Masahiko Sawada wrote:
>> Hi all,
>>
>> While testing logical replciation I found that if the transaction
>> issued DROP SUBSCRIPTION　rollbacks then the logical repliation stops
>> and the subscription can never be removed later. The document says
>> that the replication worker associated with the subscription will not
>> stop until after the transaction that issued this command has
>> committed but it doesn't work.
>
> Ah then the docs are wrong and should be fixed. Maybe we should not
> allow DROP SUBSCRIPTION inside transaction similarly to CREATE INDEX
> CONCURRENTLY.
>
>> The cause of this is that DropSubscription stops the apply worker and
>> drops corresponding replication slot on publisher side without waiting
>> for commit or rollback. The launcher process launches the apply worker
>> again but the launched worker will fail to start logical replication
>> because corresponding replication slot is already removed. And the
>> orphan subscription can not be removed later.
>>
>> I think the logical replication should not stop and the corresponding
>> replication slot and replication origin should not be removed until
>> the transaction commits.
>>
>> The solution for this I came up with is that the launcher process
>> stops the apply worker after DROP SUBSCRIPTION is committed rather
>> than DropSubscription does. And the apply worker drops replication
>> slot and replication origin before exits. Attached draft patch fixes
>> this issue.
>>
>
> I don't think we can allow the slot drop to be postponed. There is too
> many failure scenarios where we would leave the remote slot in the
> database and that's not acceptable IMHO.
>
> For example what happens if apply crashes during the DROP
> SUBSCRIPTION/COMMIT and is not started because the delete from catalog
> is now visible so the subscription is no longer there?

Another idea is to treat DROP SUBSCRIPTION in the same way as VACUUM, i.e.,
make it emit an error if it's executed within user's transaction block.
Also DROP SUBSCRIPTION should call CommitTransactionCommand() just
after removing the entry from pg_subscription, then connect to the publisher
and remove the replication slot.

Regards,

--
Fujii Masao