Re: [HACKERS] Subscription code improvements
On Thu, Aug 17, 2017 at 8:17 PM, Masahiko Sawada wrote: > On Thu, Aug 17, 2017 at 9:10 AM, Peter Eisentraut > wrote: >> On 8/8/17 05:58, Masahiko Sawada wrote: >>> Are you planning to work on remaining patches 0005 and 0006 that >>> improve the subscription codes in PG11 cycle? If not, I will take over >>> them and work on the next CF. >> >> Based on your assessment, the remaining patches were not required bug >> fixes. So I think preparing them for the next commit fest would be great. >> > > Thank you for the comment. > > After more thought, since 0001 and 0003 patches on the first mail also > improve the subscription codes and are worth to be considered, I > picked total 4 patches up and updated them. I'm planning to work on > these patches in the next CF. > Added this item to the next commit fest. 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
Re: [HACKERS] Subscription code improvements
On Thu, Aug 17, 2017 at 9:10 AM, Peter Eisentraut wrote: > On 8/8/17 05:58, Masahiko Sawada wrote: >> Are you planning to work on remaining patches 0005 and 0006 that >> improve the subscription codes in PG11 cycle? If not, I will take over >> them and work on the next CF. > > Based on your assessment, the remaining patches were not required bug > fixes. So I think preparing them for the next commit fest would be great. > Thank you for the comment. After more thought, since 0001 and 0003 patches on the first mail also improve the subscription codes and are worth to be considered, I picked total 4 patches up and updated them. I'm planning to work on these patches in the next CF. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center 0001-Improve-messaging-during-logical-replication-worker-_v2.patch Description: Binary data 0002-Split-the-SetSubscriptionRelState-function-into-two_v2.patch Description: Binary data 0003-Allow-syscache-access-to-subscriptions-in-database-l_v2.patch Description: Binary data 0004-Improve-locking-for-subscriptions-and-subscribed-rel_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On 8/8/17 05:58, Masahiko Sawada wrote: > Are you planning to work on remaining patches 0005 and 0006 that > improve the subscription codes in PG11 cycle? If not, I will take over > them and work on the next CF. Based on your assessment, the remaining patches were not required bug fixes. So I think preparing them for the next commit fest would be great. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
Petr, On Thu, Aug 3, 2017 at 5:24 AM, Stephen Frost wrote: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Alvaro Herrera writes: >> > I wish you'd stop splitting error message strings across multiple lines. >> > I've been trapped by a faulty grep not matching a split error message a >> > number of times :-( I know by now to remove words until I get a match, >> > but it seems an unnecessary trap for the unwary. >> >> Yeah, that's my number one reason for not splitting error messages, too. >> It's particularly nasty if similar strings appear in multiple places and >> they're not all split alike, as you can get misled into thinking that a >> reported error must have occurred in a place you found, rather than >> someplace you didn't. > > +1. > Are you planning to work on remaining patches 0005 and 0006 that improve the subscription codes in PG11 cycle? If not, I will take over them and work on the next CF. 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
Re: [HACKERS] Subscription code improvements
On Mon, Aug 7, 2017 at 10:22 PM, Peter Eisentraut wrote: > On 8/7/17 00:27, Masahiko Sawada wrote: 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. >>> >>> Maybe we can just remove those messages? >>> >>> We don't get messages when we create a subscription about which tables >>> are part of it. So why do we need such messages when we refresh a >>> subscription? >> >> I think that the messages is useful when we add/remove tables to/from >> the publication and then refresh the subscription, so we might want to >> change it to DEBUG rather than elimination. > > Good idea. Done that way. > Thank you! 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
Re: [HACKERS] Subscription code improvements
On 8/7/17 00:27, Masahiko Sawada wrote: >>> 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. >> >> Maybe we can just remove those messages? >> >> We don't get messages when we create a subscription about which tables >> are part of it. So why do we need such messages when we refresh a >> subscription? > > I think that the messages is useful when we add/remove tables to/from > the publication and then refresh the subscription, so we might want to > change it to DEBUG rather than elimination. Good idea. Done that way. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On 8/7/17 00:32, Masahiko Sawada wrote: > On Sun, Aug 6, 2017 at 7:44 AM, Noah Misch wrote: >> On Wed, Aug 02, 2017 at 04:09:43PM -0400, Peter Eisentraut wrote: >>> On 8/1/17 00:17, Noah Misch wrote: The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. >>> >>> I'm looking into this now and will report by Friday. >> >> This PostgreSQL 10 open item is past due for your status update. Kindly send >> a status update within 24 hours, and include a date for your subsequent >> status >> update. Refer to the policy on open item ownership: >> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > I think this open item has closed by the commit > 7e174fa793a2df89fe03d002a5087ef67abcdde8 ? Yes. I have updated the wiki page now. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On Sun, Aug 6, 2017 at 7:44 AM, Noah Misch wrote: > On Wed, Aug 02, 2017 at 04:09:43PM -0400, Peter Eisentraut wrote: >> On 8/1/17 00:17, Noah Misch wrote: >> > The above-described topic is currently a PostgreSQL 10 open item. Peter, >> > since you committed the patch believed to have created it, you own this >> > open >> > item. If some other commit is more relevant or if this does not belong as >> > a >> > v10 open item, please let us know. Otherwise, please observe the policy on >> > open item ownership[1] and send a status update within three calendar days >> > of >> > this message. Include a date for your subsequent status update. Testers >> > may >> > discover new open items at any time, and I want to plan to get them all >> > fixed >> > well in advance of shipping v10. Consequently, I will appreciate your >> > efforts >> > toward speedy resolution. Thanks. >> >> I'm looking into this now and will report by Friday. > > This PostgreSQL 10 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com I think this open item has closed by the commit 7e174fa793a2df89fe03d002a5087ef67abcdde8 ? 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
Re: [HACKERS] Subscription code improvements
On Sat, Aug 5, 2017 at 10:29 AM, Peter Eisentraut wrote: > On 8/4/17 12:02, Masahiko Sawada wrote: >> 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. > > OK, I have committed the 0004 patch. Thank you! > >> 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. > > Maybe we can just remove those messages? > > We don't get messages when we create a subscription about which tables > are part of it. So why do we need such messages when we refresh a > subscription? I think that the messages is useful when we add/remove tables to/from the publication and then refresh the subscription, so we might want to change it to DEBUG rather than elimination. 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
Re: [HACKERS] Subscription code improvements
On Wed, Aug 02, 2017 at 04:09:43PM -0400, Peter Eisentraut wrote: > On 8/1/17 00:17, Noah Misch wrote: > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > > since you committed the patch believed to have created it, you own this open > > item. If some other commit is more relevant or if this does not belong as a > > v10 open item, please let us know. Otherwise, please observe the policy on > > open item ownership[1] and send a status update within three calendar days > > of > > this message. Include a date for your subsequent status update. Testers > > may > > discover new open items at any time, and I want to plan to get them all > > fixed > > well in advance of shipping v10. Consequently, I will appreciate your > > efforts > > toward speedy resolution. Thanks. > > I'm looking into this now and will report by Friday. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On 8/4/17 12:02, Masahiko Sawada wrote: > 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. OK, I have committed the 0004 patch. > 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. Maybe we can just remove those messages? We don't get messages when we create a subscription about which tables are part of it. So why do we need such messages when we refresh a subscription? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On Fri, Aug 4, 2017 at 2:17 AM, Peter Eisentraut 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
Re: [HACKERS] Subscription code improvements
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?) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Alvaro Herrera writes: > > I wish you'd stop splitting error message strings across multiple lines. > > I've been trapped by a faulty grep not matching a split error message a > > number of times :-( I know by now to remove words until I get a match, > > but it seems an unnecessary trap for the unwary. > > Yeah, that's my number one reason for not splitting error messages, too. > It's particularly nasty if similar strings appear in multiple places and > they're not all split alike, as you can get misled into thinking that a > reported error must have occurred in a place you found, rather than > someplace you didn't. +1. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Subscription code improvements
Alvaro Herrera writes: > I wish you'd stop splitting error message strings across multiple lines. > I've been trapped by a faulty grep not matching a split error message a > number of times :-( I know by now to remove words until I get a match, > but it seems an unnecessary trap for the unwary. Yeah, that's my number one reason for not splitting error messages, too. It's particularly nasty if similar strings appear in multiple places and they're not all split alike, as you can get misled into thinking that a reported error must have occurred in a place you found, rather than someplace you didn't. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
Petr Jelinek wrote: > I split it into several patches: I wish you'd stop splitting error message strings across multiple lines. I've been trapped by a faulty grep not matching a split error message a number of times :-( I know by now to remove words until I get a match, but it seems an unnecessary trap for the unwary. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On 8/1/17 00:17, Noah Misch wrote: > The above-described topic is currently a PostgreSQL 10 open item. Peter, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. I'm looking into this now and will report by Friday. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On Fri, Jul 07, 2017 at 10:19:19PM +0200, Petr Jelinek wrote: > 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 [1]). > 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. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Subscription code improvements
On Wed, Jul 12, 2017 at 11:19 AM, Masahiko Sawada wrote: > On Sat, Jul 8, 2017 at 5:19 AM, Petr Jelinek > 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 [1]). > > 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 [2] 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. I've reviewed 0002, 0004 and 0005 patches briefly. Here are some comments. -- 0002-Exit-in-sync-worker-if-relation-was-removed-during-s.patch As Petr mentioned, if the table subscription is removed before the table sync worker gets the subscription relation entry, GetSubscriptionRelState could returns SUBREL_STATE_UNKNOWN and this issue happens. Actually we now can handle this case properly by commit 8dc7c338129d22a52d4afcf2f83a73041119efda. So this seems to be an improvement and not be a release blocker. Therefore for this patch, I think it's better to do ereport in the switch(MyLogicalRepWorker->relstate) block. BTW, since missing_ok of GetSubscriptionRelState() is always set to true I think that we can remove it. Also because the reason I mention at later part this fix might not be necessary. -- 0004-Only-kill-sync-workers-at-commit-time-in-SUBSCRIPTIO.patch + /* +* If we are dropping slot, stop all the subscription workers immediately +* so that the slot is accessible, otherwise just shedule the stop at the +* end of the transaction. +* +* New workers won't be started because we hold exclusive lock on the +* subscription till the end of transaction. +*/ + LWLockAcquire(LogicalRepWorkerLock, LW_SHARED); + subworkers = logicalrep_sub_workers_find(subid, false); + LWLockRelease(LogicalRepWorkerLock); + foreach (lc, subworkers) + { + LogicalRepWorker *w = (LogicalRepWorker *) lfirst(lc); + if (sub->slotname) + logicalrep_worker_stop(w->subid, w->relid); + else + logicalrep_worker_stop_at_commit(w->subid, w->relid); + } + list_free(subworkers); I think if we kill the all workers including table sync workers then the fix by 0002 patch is not necessary actually, because the table sync worker will not see that the subscription relation state has been removed. Also, logicalrep_sub_workers_find() function is defined in 0006 patch but it would be better to move it to 0004 patch. -- 0005-Allow-syscache-access-to-subscriptions-in-database-l.patch Do we need to update the comment at the top of IndexScanOK()? To summary, I think we now have only one issue; ALTER SUBSCRIPTION is not transactional, 0004 patch is addressing this issue . 0002 patch seems an improvement patch to me, and it might be resolved by 0004 patch. 0005 patch is required by 0006 patch which is an improvement patch. Am I missing something? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source
Re: [HACKERS] Subscription code improvements
On Sat, Jul 8, 2017 at 5:19 AM, Petr Jelinek 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 [1]). 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 [2] 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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers