On 15/02/17 06:43, Masahiko Sawada wrote:
> On Tue, Feb 14, 2017 at 1:13 AM, Fujii Masao <masao.fu...@gmail.com> wrote:
>> On Mon, Feb 13, 2017 at 4:05 PM, Masahiko Sawada <sawada.m...@gmail.com> 
>> wrote:
>>> On Sat, Feb 11, 2017 at 8:18 PM, Petr Jelinek
>>> <petr.jeli...@2ndquadrant.com> wrote:
>>>> On 10/02/17 19:55, Masahiko Sawada wrote:
>>>>> On Thu, Feb 9, 2017 at 12:44 AM, Petr Jelinek
>>>>> <petr.jeli...@2ndquadrant.com> wrote:
>>>>>> On 08/02/17 07:40, Masahiko Sawada wrote:
>>>>>>> On Wed, Feb 8, 2017 at 9:01 AM, Michael Paquier
>>>>>>> <michael.paqu...@gmail.com> wrote:
>>>>>>>> On Wed, Feb 8, 2017 at 1:30 AM, Fujii Masao <masao.fu...@gmail.com> 
>>>>>>>> wrote:
>>>>>>>>> On Wed, Feb 8, 2017 at 12:26 AM, Petr Jelinek
>>>>>>>>> <petr.jeli...@2ndquadrant.com> wrote:
>>>>>>>>>> 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.
>>>>>>>> It seems to me that this is exactly Petr's point: using
>>>>>>>> PreventTransactionChain() to prevent things to happen.
>>>>>>> Agreed. It's better to prevent to be executed inside user transaction
>>>>>>> block. And I understood there is too many failure scenarios we need to
>>>>>>> handle.
>>>>>>>>> Also DROP SUBSCRIPTION should call CommitTransactionCommand() just
>>>>>>>>> after removing the entry from pg_subscription, then connect to the 
>>>>>>>>> publisher
>>>>>>>>> and remove the replication slot.
>>>>>>>> For consistency that may be important.
>>>>>>> Agreed.
>>>>>>> Attached patch, please give me feedback.
>>>>>> This looks good (and similar to what initial patch had btw). Works fine
>>>>>> for me as well.
>>>>>> Remaining issue is, what to do about CREATE SUBSCRIPTION then, there are
>>>>>> similar failure scenarios there, should we prevent it from running
>>>>>> inside transaction as well?
>>>>> Hmm,  after thought I suspect current discussing approach. For
>>>>> example, please image the case where CRAETE SUBSCRIPTION creates
>>>>> subscription successfully but fails to create replication slot for
>>>>> whatever reason, and then DROP SUBSCRIPTION drops the subscription but
>>>>> dropping replication slot is failed. In such case, CREAET SUBSCRIPTION
>>>>> and DROP SUBSCRIPTION return ERROR but the subscription is created and
>>>>> dropped successfully. I think that this behaviour confuse the user.
>>>>> I think we should just prevent calling DROP SUBSCRIPTION in user's
>>>>> transaction block. Or I guess that it could be better to separate the
>>>>> starting/stopping logical replication from subscription management.
>>>> We need to stop the replication worker(s) in order to be able to drop
>>>> the slot. There is no such issue with startup of the worker as that one
>>>> is launched by launcher after the transaction has committed.
>>>> IMO best option is to just don't allow DROP/CREATE SUBSCRIPTION inside a
>>>> transaction block and don't do any commits inside of those (so that
>>>> there are no rollbacks, which solves your initial issue I believe). That
>>>> way failure to create/drop slot will result in subscription not being
>>>> created/dropped which is what we want.
>> On second thought, +1.
>>> I basically agree this option, but why do we need to change CREATE
>>> SUBSCRIPTION as well?
>> Because the window between the creation of replication slot and the 
>> transaction
>> commit of CREATE SUBSCRIPTION should be short. Otherwise, if any error 
>> happens
>> during that window, the replication slot unexpectedly remains while there is 
>> no
>> corresponding subscription. Of course, even If we prevent CREATE SUBSCRIPTION
>> from being executed within user's transaction block, there is still such
>> window. But we can reduce the possibility of that problem.
> Thank you for the explanation. I understood and agree.
> I think we should disallow to call ALTER SUBSCRIPTION inside a user's
> transaction block as well.

Why? ALTER SUBSCRIPTION does not create/drop anything on remote server ever.

  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to