On Fri, Aug 4, 2017 at 2:17 AM, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > On 7/13/17 23:53, Masahiko Sawada wrote: >> To summary, I think we now have only one issue; ALTER SUBSCRIPTION is >> not transactional, 0004 patch is addressing this issue . > > We have two competing patches for this issue. This patch moves the > killing to the end of the DDL transaction. Your earlier patch makes the > tablesync work itself responsible for exiting. Do you wish to comment > which direction to pursue? (Doing both might also be an option?) >
To make ALTER SUBSCRIPTION REFRESH being transactional, I prefer Petr's proposal. Because it can make ALTER SUBSCRIPTION and DROP SUBSCRIPTION stop the table sync workers that are in progress of copying data. I'm not sure killing table sync workers in DDL commands would be acceptable but since it can free unnecessary slots of logical replication workers and replication slots I'd prefer this idea. However, even with this patch there is still an issue that NOTICE messages "removed subscription for table public.t1" can be appeared even if we rollback ALTER SUBSCRIPTION REFRESH command as I mentioned on earlier thread. Since I think this behaviour will confuse users who check server logs I'd like to deal with it, I don't have a good idea though. Also, I think we can incorporate the idea of my earlier proposal with some changes (i.g. I'd choose the third option). In current implementation, in case where a subscription relation state is accidentally removed while the corresponding table sync worker is progress of copying data, it cannot exit from a loop in wait_for_worker_state_change unless the apply worker dies. So to be more robust, table sync workers can finish with an error if its subscription relation state has disappeared. 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