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. >
I basically agree this option, but why do we need to change CREATE SUBSCRIPTION as well? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
