On Sat, Jul 8, 2017 at 5:19 AM, Petr Jelinek <petr.jeli...@2ndquadrant.com> wrote: > Hi, > > I have done some review of subscription handling (well self-review) and > here is the result of that (It's slightly improved version from another > thread ).
Thank you for working on this and patches! > I split it into several patches: > > 0001 - Makes subscription worker exit nicely when there is subscription > missing (ie was removed while it was starting) and also makes disabled > message look same as the message when disabled state was detected while > worker is running. This is mostly just nicer behavior for user (ie no > unexpected errors in log when you just disabled the subscription). > > 0002 - This is bugfix - the sync worker should exit when waiting for > apply and apply dies otherwise there is possibility of not being > correctly synchronized. > > 0003 - Splits the schizophrenic SetSubscriptionRelState function into > two which don't try to do broken upsert and report proper errors instead. > > 0004 - Solves the issue which Masahiko Sawada reported  about ALTER > SUBSCRIPTION handling of workers not being transactional - we now do > killing of workers transactionally (and we do the same when possible in > DROP SUBSCRIPTION). > > 0005 - Bugfix to allow syscache access to subscription without being > connected to database - this should work since subscription is pinned > catalog, it wasn't caught because nothing in core is using it (yet). > > 0006 - Makes the locking of subscriptions more granular (this depends on > 0005). This removes the ugly AccessExclusiveLock on the pg_subscription > catalog from DROP SUBSCRIPTION and also solves some potential race > conditions between launcher, ALTER SUBSCRIPTION and > process_syncing_tables_for_apply(). Also simplifies memory handling in > launcher as well as logicalrep_worker_stop() function. This basically > makes subscriptions behave like every other object in the database in > terms of locking. > > Only the 0002, 0004 and 0005 are actual bug fixes, but I'd still like to > get it all into PG10 as especially the locking now behaves really > differently than everything else and that does not seem like a good idea. > I'm now planing to review 0002, 0004 and 0005 patches first as they are bug fixes. Should we add them to the open item list? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers