)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"15b31d3c78771a38e2f79d088f61ea25401fee1b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Oleg Bondarev \u003cobondarev@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-08-07 20:17:07 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure l3 agent receives notification about added router"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently router_added (and other) notifications are sent"},{"line_number":10,"context_line":"to agents with an RPC cast() method which does not ensure that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1a4dcd0f_22926c58","line":7,"updated":"2015-08-09 14:04:52.000000000","message":"In the code you\u0027ve changed specifically router_added to call but not router_updated or router_removed, nor the manual rescheduling notifications. Why is that? I\u0027m not sure I understand the rationale. (By the way it\u0027s pretty weird to me that those are all casts).","commit_id":"08a30f170d7e5905942d34481e100143f936f03d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c0e8f8cf91993b2ecf1f11085ce3a89c11ce3361","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Oleg Bondarev \u003cobondarev@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-08-07 20:17:07 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure l3 agent receives notification about added router"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently router_added (and other) notifications are sent"},{"line_number":10,"context_line":"to agents with an RPC cast() method which does not ensure that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1a4dcd0f_5495e43d","line":7,"in_reply_to":"1a4dcd0f_22926c58","updated":"2015-08-10 08:40:36.000000000","message":"The rationale is simple: the bug occured in the fields - that\u0027s why I\u0027ve changed only router_added here (BTW manual rescheduling is using the same notification). Not sure router_removed should use call() because it\u0027s often sent to offline agents (auto rescheduling). My proposal would be to not try to fix everything in one patch, but to use case-by-case tactic with a clear rationale for each notification. Please let me know what you think about it.","commit_id":"08a30f170d7e5905942d34481e100143f936f03d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"16ea78beb5cab588016230c8af017a04270c45ea","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Ensure l3 agent receives notification about added router"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently router_added (and other) notifications are sent"},{"line_number":10,"context_line":"to agents with an RPC cast() method which does not ensure that"},{"line_number":11,"context_line":"the message is actually delivered to the recipient."},{"line_number":12,"context_line":"If the message is lost (due to instability of messaging system"},{"line_number":13,"context_line":"in some failover scenarios for example) neither server nor agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"da20952f_cc23c257","line":10,"updated":"2015-09-08 14:39:50.000000000","message":"I believe one of the main points of putting a message queue component into a distributed system is so you can hand over the responsibility of message delivery to the MQ. And cast() can be made reliable.\n\nI would argue that if your MQ can\u0027t guarantee one-way message delivery then it\u0027s either misconfigured or buggy. If we start guaranteeing message delivery from client code that kind of defeats having an MQ in the first place. Then we could just send plain UDP messages.","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"0ffe0069709182327348d1e110b743307af8cd4d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Ensure l3 agent receives notification about added router"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently router_added (and other) notifications are sent"},{"line_number":10,"context_line":"to agents with an RPC cast() method which does not ensure that"},{"line_number":11,"context_line":"the message is actually delivered to the recipient."},{"line_number":12,"context_line":"If the message is lost (due to instability of messaging system"},{"line_number":13,"context_line":"in some failover scenarios for example) neither server nor agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"da20952f_c9162f58","line":10,"in_reply_to":"da20952f_3b3ab199","updated":"2015-09-09 10:54:23.000000000","message":"I was thinking on alternatives since yesterday. I understand if you need a workaround for a problem right now and I don\u0027t want to block that. So I took off even the -1.\n\nI have two ideas/suggestions:\n\nI guess we are talking about rabbitmq losing messages during failover. But do have that reported as a bug? Let\u0027s suppose we report it and somebody fixes it. Then do we still need this workaround?\n\nAs a longer term fix what do you think about the following?\n\nIt seems we don\u0027t care about which l3-agent creates the router as long as we can place it on at least one agent successfully. Today we have to check if the placement was successful, because we first decide which agent should take it (ie. we schedule the agent) and then we try to put it there actually. But this could be done by the MQ. We can setup exchanges and queues in such a way, that we send one message (a cast). It is addressed to a group (ie. a type) of agents. But it will not fan out, will not be delivered in multiple copies to all agent instances. Instead exactly one copy can be delivered to one agent and the MQ can decide which one. Therefore as long as we have at least one agent functioning in the system, that will get the message and can implement the router.\n\nIf neutron-server needs to know (does it) which agent has got the router, it can learn from the next periodic report. I believe the rudimentary load balancing achieved by agent scheduling can be reproduced in the MQ (for example a round robin allocation).\n\nThis is a messaging pattern I have not seen in neutron yet, but it\u0027s definitely possible.\n\nTo your question about call: I believe there\u0027s a still a difference between call and a request-response pair of casts. That is the latter can be done without blocking on either side. So you use call when you can\u0027t progress locally without an answer from a remote system. But here I\u0027m not sure if we are in that scenario.","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"bc185dc1b94f643f8350ddea0ba8fae7b04862c0","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Ensure l3 agent receives notification about added router"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently router_added (and other) notifications are sent"},{"line_number":10,"context_line":"to agents with an RPC cast() method which does not ensure that"},{"line_number":11,"context_line":"the message is actually delivered to the recipient."},{"line_number":12,"context_line":"If the message is lost (due to instability of messaging system"},{"line_number":13,"context_line":"in some failover scenarios for example) neither server nor agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"ba15a1d1_e7c6a948","line":10,"in_reply_to":"da20952f_c9162f58","updated":"2015-09-15 09:10:44.000000000","message":"There are a few points for a good bikeshedding cum yak shaving session here.\n\nCan we assume that our message transport layer is reliable? We are implicitly doing that now, but I reckon we should not.\n\nIs it ok to leverage concepts from the MQ layer to do a more efficient message distribution? I\u0027d say probably not, unless we can find abstractions common to all MQ systems we support; note also that is probably tantamount to delegate scheduling to the MQ layer, and I don\u0027t know how we could implement there logic such as that for allocating a router on the agent which is less loaded.\n\nFinally, on call vs casting two ways; I tend to slight prefer call in this case, if nothing else because it leads to a shorter code change. If we add a \"callback cast\" method then we\u0027ll also have to worry about versioning of the RPC interface I think","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"8bd4b5f70e3863376c952e5cda9d4fa6e3715a07","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Ensure l3 agent receives notification about added router"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently router_added (and other) notifications are sent"},{"line_number":10,"context_line":"to agents with an RPC cast() method which does not ensure that"},{"line_number":11,"context_line":"the message is actually delivered to the recipient."},{"line_number":12,"context_line":"If the message is lost (due to instability of messaging system"},{"line_number":13,"context_line":"in some failover scenarios for example) neither server nor agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"da20952f_3b3ab199","line":10,"in_reply_to":"da20952f_cc23c257","updated":"2015-09-08 16:18:34.000000000","message":"I agree and yes, our message system is buggy and in fact one of the most painful components ATM. I\u0027m sure folks are working on it to make it better. I think we need to deal with it however, and try to improve things from neutron side as much as we can.\n\nThe question would be: if cast() is reliable (ideal world) and caller code should not worry about it why do we need call() at all?","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"1d601d3bcebb28cc455a41aa857de2e52fd1e2c5","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This also unifies reschedule_router() method to avoid code duplication"},{"line_number":21,"context_line":"between legacy and dvr agent schedulers."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Closes-Bug: #1482630"},{"line_number":24,"context_line":"Related-Bug #1404743"},{"line_number":25,"context_line":"Change-Id: Id08764ba837d8f47a28649d081a5876797fe369e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"da85f559_962a8e7d","line":22,"updated":"2015-11-13 11:56:31.000000000","message":"I think that this requires a UpgradeImpact flag","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"4c04a8663e94da2e10fa0a99c2b28fa35a886e73","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This also unifies reschedule_router() method to avoid code duplication"},{"line_number":21,"context_line":"between legacy and dvr agent schedulers."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Closes-Bug: #1482630"},{"line_number":24,"context_line":"Related-Bug #1404743"},{"line_number":25,"context_line":"Change-Id: Id08764ba837d8f47a28649d081a5876797fe369e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"da85f559_96860e64","line":22,"in_reply_to":"da85f559_962a8e7d","updated":"2015-11-13 12:02:59.000000000","message":"why?","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"}],"neutron/api/rpc/agentnotifiers/l3_rpc_agent_api.py":[{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"5959e677f0f06e9b1290ebad68bacc9f34dd2474","unresolved":false,"context_lines":[{"line_number":45,"context_line":"                  \u0027%(method)s\u0027, {\u0027host\u0027: host,"},{"line_number":46,"context_line":"                                 \u0027method\u0027: method})"},{"line_number":47,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost)"},{"line_number":48,"context_line":"        rpc_method \u003d cctxt.call if use_call else cctxt.cast"},{"line_number":49,"context_line":"        rpc_method(context, method, payload\u003dpayload)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _agent_notification(self, context, method, router_ids, operation,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a4dcd0f_03712d44","line":48,"updated":"2015-08-17 06:01:02.000000000","message":"In case of failure how the information will be passed to the end user? Does cctxt.call is a blocking call?","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"0d64973c6a9dca34bb7810189128d322c87e3d2d","unresolved":false,"context_lines":[{"line_number":45,"context_line":"                  \u0027%(method)s\u0027, {\u0027host\u0027: host,"},{"line_number":46,"context_line":"                                 \u0027method\u0027: method})"},{"line_number":47,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost)"},{"line_number":48,"context_line":"        rpc_method \u003d cctxt.call if use_call else cctxt.cast"},{"line_number":49,"context_line":"        rpc_method(context, method, payload\u003dpayload)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _agent_notification(self, context, method, router_ids, operation,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a4dcd0f_f98dced3","line":48,"in_reply_to":"1a4dcd0f_03712d44","updated":"2015-08-17 10:18:06.000000000","message":"\u003e\u003eIn case of failure how the information will be passed to the end user?\n it depends on error handling at higher levels\n \u003e\u003eDoes cctxt.call is a blocking call?\n it is","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"ec4e5dbda338ab20f397f5ab5cebd52d8c591deb","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def router_added_to_agent(self, context, router_ids, host):"},{"line_number":162,"context_line":"        # need to use call here as we want to be sure agent received"},{"line_number":163,"context_line":"        # notification and router will not be \"lost\""},{"line_number":164,"context_line":"        self._notification_host(context, \u0027router_added_to_agent\u0027,"},{"line_number":165,"context_line":"                                router_ids, host, use_call\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_18a3c101","line":163,"updated":"2015-08-28 18:47:17.000000000","message":"A call() can lost just as easily as a cast(), so I think this comment is misleading.  Of course, if it is lost, you\u0027ll get an exception after a timeout is reached waiting for the response.\n\nHowever, even if you *do* get an exception, it doesn\u0027t mean the message wasn\u0027t received!  It just means you didn\u0027t get the response within a timeout.  It could be that the response got lost.  It could be that the server side is overloaded and is taking too long to process the message, or messages in general.","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"ea87db6aec6bebd490960d591bab9130a254304e","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def router_added_to_agent(self, context, router_ids, host):"},{"line_number":162,"context_line":"        # need to use call here as we want to be sure agent received"},{"line_number":163,"context_line":"        # notification and router will not be \"lost\""},{"line_number":164,"context_line":"        self._notification_host(context, \u0027router_added_to_agent\u0027,"},{"line_number":165,"context_line":"                                router_ids, host, use_call\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_1e7fbfe0","line":163,"in_reply_to":"da20952f_18a3c101","updated":"2015-09-01 10:38:57.000000000","message":"Well, the comment says \u0027we want to be sure\u0027 meaning that generally call() is more reliable than cast(). it doesn\u0027t say that call() itself ensures that router will not be lost. Will try to rephrase to make it more clear.\n\nAgree with your second point but I don\u0027t think it changes anything for us, if it was notification or just a response that was lost: we still need to handle it as notification failure.","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"a23ee9577e7d38ee7b2cec004eb0516db71e229b","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        self._notification_host(context, \u0027router_removed_from_agent\u0027, host,"},{"line_number":158,"context_line":"                                payload\u003d{\u0027router_id\u0027: router_id})"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def router_added_to_agent(self, context, router_ids, host):"},{"line_number":161,"context_line":"        # need to use call here as we want to be sure agent received"},{"line_number":162,"context_line":"        # notification and router will not be \"lost\". However using call()"},{"line_number":163,"context_line":"        # itself is not a guarantee, calling code should handle exceptions and"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a26ad4f_dbc69967","line":160,"updated":"2015-11-02 06:36:58.000000000","message":"why just router_added_to_agent use call, router_removed_from_agent and routers_updated_on_host not use?","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"70b2bb996cd5fd9ce51dba65501fb77724ce986b","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        self._notification_host(context, \u0027router_removed_from_agent\u0027, host,"},{"line_number":158,"context_line":"                                payload\u003d{\u0027router_id\u0027: router_id})"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def router_added_to_agent(self, context, router_ids, host):"},{"line_number":161,"context_line":"        # need to use call here as we want to be sure agent received"},{"line_number":162,"context_line":"        # notification and router will not be \"lost\". However using call()"},{"line_number":163,"context_line":"        # itself is not a guarantee, calling code should handle exceptions and"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_4b36c6b5","line":160,"in_reply_to":"1a26ad4f_dbc69967","updated":"2015-11-13 08:29:29.000000000","message":"I\u0027d like to limit the scope to particular bug","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"1d601d3bcebb28cc455a41aa857de2e52fd1e2c5","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        # itself is not a guarantee, calling code should handle exceptions and"},{"line_number":164,"context_line":"        # retry"},{"line_number":165,"context_line":"        self._notification_host(context, \u0027router_added_to_agent\u0027, host,"},{"line_number":166,"context_line":"                                use_call\u003dTrue, payload\u003drouter_ids)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def routers_updated_on_host(self, context, router_ids, host):"},{"line_number":169,"context_line":"        self._notification_host(context, \u0027routers_updated\u0027, host,"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_d3e1089c","line":166,"updated":"2015-11-13 11:56:31.000000000","message":"this will break an existing setup if the neutron service is upgraded and the agents are not.","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d83c67a95540558e7b5b66191b03ae201a170dbf","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        # itself is not a guarantee, calling code should handle exceptions and"},{"line_number":164,"context_line":"        # retry"},{"line_number":165,"context_line":"        self._notification_host(context, \u0027router_added_to_agent\u0027, host,"},{"line_number":166,"context_line":"                                use_call\u003dTrue, payload\u003drouter_ids)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def routers_updated_on_host(self, context, router_ids, host):"},{"line_number":169,"context_line":"        self._notification_host(context, \u0027routers_updated\u0027, host,"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba8a016a_fc510437","line":166,"in_reply_to":"da85f559_16da1e79","updated":"2015-11-18 17:16:23.000000000","message":"ok, understood. so ignore my comment in the commit message","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"4c04a8663e94da2e10fa0a99c2b28fa35a886e73","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        # itself is not a guarantee, calling code should handle exceptions and"},{"line_number":164,"context_line":"        # retry"},{"line_number":165,"context_line":"        self._notification_host(context, \u0027router_added_to_agent\u0027, host,"},{"line_number":166,"context_line":"                                use_call\u003dTrue, payload\u003drouter_ids)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def routers_updated_on_host(self, context, router_ids, host):"},{"line_number":169,"context_line":"        self._notification_host(context, \u0027routers_updated\u0027, host,"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_16da1e79","line":166,"in_reply_to":"da85f559_d3e1089c","updated":"2015-11-13 12:02:59.000000000","message":"actually we do not need to upgrade agents in this case. Handling on agent side is not changed. Am I missing smth?","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"432faa17da4e35dc075627b3907e3731ab6bb4a3","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def routers_updated_on_host(self, context, router_ids, host):"},{"line_number":169,"context_line":"        self._notification_host(context, \u0027routers_updated\u0027, host,"},{"line_number":170,"context_line":"                                routers\u003drouter_ids)"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a26ad4f_05411938","line":170,"updated":"2015-11-01 12:55:39.000000000","message":"how about passing \"payload\u003drouter_ids\" for consistency","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"70b2bb996cd5fd9ce51dba65501fb77724ce986b","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def routers_updated_on_host(self, context, router_ids, host):"},{"line_number":169,"context_line":"        self._notification_host(context, \u0027routers_updated\u0027, host,"},{"line_number":170,"context_line":"                                routers\u003drouter_ids)"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_ab8c1280","line":170,"in_reply_to":"1a26ad4f_05411938","updated":"2015-11-13 08:29:29.000000000","message":"l3 agent expects \u0027routers\u0027 parameter","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"}],"neutron/db/l3_agentschedulers_db.py":[{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"824476361fcaae660b306774e49a7ae5c807557c","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                l3_notifier.router_added_to_agent("},{"line_number":285,"context_line":"                    context, [router_id], new_agent.host)"},{"line_number":286,"context_line":"            except oslo_messaging.MessagingException:"},{"line_number":287,"context_line":"                max_attempts \u003d 5"},{"line_number":288,"context_line":"                LOG.warning(_LW(\u0027Failed to notify L3 agent on host %(host)s \u0027"},{"line_number":289,"context_line":"                                \u0027about added router. Attempt %(attempt)d out \u0027"},{"line_number":290,"context_line":"                                \u0027of %(max_attempts)d\u0027),"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a4dcd0f_d706a884","line":287,"updated":"2015-08-16 17:46:56.000000000","message":"IMHO, let\u0027s avoid hard coded values. We can use constants instead.","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"dfc71c2f95292b932cf94d20127d2cbd0a0b5cb3","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                l3_notifier.router_added_to_agent("},{"line_number":285,"context_line":"                    context, [router_id], new_agent.host)"},{"line_number":286,"context_line":"            except oslo_messaging.MessagingException:"},{"line_number":287,"context_line":"                max_attempts \u003d 5"},{"line_number":288,"context_line":"                LOG.warning(_LW(\u0027Failed to notify L3 agent on host %(host)s \u0027"},{"line_number":289,"context_line":"                                \u0027about added router. Attempt %(attempt)d out \u0027"},{"line_number":290,"context_line":"                                \u0027of %(max_attempts)d\u0027),"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa1b9901_5f818349","line":287,"in_reply_to":"1a4dcd0f_d706a884","updated":"2015-08-17 23:49:57.000000000","message":"+1, i agree i would prefer to avoid hardcoded value even more in the middle of a method","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"0d64973c6a9dca34bb7810189128d322c87e3d2d","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                l3_notifier.router_added_to_agent("},{"line_number":285,"context_line":"                    context, [router_id], new_agent.host)"},{"line_number":286,"context_line":"            except oslo_messaging.MessagingException:"},{"line_number":287,"context_line":"                max_attempts \u003d 5"},{"line_number":288,"context_line":"                LOG.warning(_LW(\u0027Failed to notify L3 agent on host %(host)s \u0027"},{"line_number":289,"context_line":"                                \u0027about added router. Attempt %(attempt)d out \u0027"},{"line_number":290,"context_line":"                                \u0027of %(max_attempts)d\u0027),"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a4dcd0f_f4e63543","line":287,"in_reply_to":"1a4dcd0f_d706a884","updated":"2015-08-17 10:18:06.000000000","message":"this can be considered as a constant, currently I don\u0027t see a reason for making it a module level constant or config parameter.","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"273c001c1ecbdf5c4dcb048f84c40d0b8bc7f8a0","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                l3_notifier.router_added_to_agent("},{"line_number":285,"context_line":"                    context, [router_id], new_agent.host)"},{"line_number":286,"context_line":"            except oslo_messaging.MessagingException:"},{"line_number":287,"context_line":"                max_attempts \u003d 5"},{"line_number":288,"context_line":"                LOG.warning(_LW(\u0027Failed to notify L3 agent on host %(host)s \u0027"},{"line_number":289,"context_line":"                                \u0027about added router. Attempt %(attempt)d out \u0027"},{"line_number":290,"context_line":"                                \u0027of %(max_attempts)d\u0027),"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa1b9901_672b91ec","line":287,"in_reply_to":"fa1b9901_5f818349","updated":"2015-08-18 06:53:15.000000000","message":"so what would you propose? Module level constant?","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"dfc71c2f95292b932cf94d20127d2cbd0a0b5cb3","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                    context, [router_id], new_agent.host)"},{"line_number":286,"context_line":"            except oslo_messaging.MessagingException:"},{"line_number":287,"context_line":"                max_attempts \u003d 5"},{"line_number":288,"context_line":"                LOG.warning(_LW(\u0027Failed to notify L3 agent on host %(host)s \u0027"},{"line_number":289,"context_line":"                                \u0027about added router. Attempt %(attempt)d out \u0027"},{"line_number":290,"context_line":"                                \u0027of %(max_attempts)d\u0027),"},{"line_number":291,"context_line":"                            {\u0027host\u0027: new_agent.host, \u0027attempt\u0027: attempt,"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa1b9901_1fcc6b88","line":288,"updated":"2015-08-17 23:49:57.000000000","message":"The current implementation is quite difficult to understand (and test?). IMO, separating the retry mechanism from the \"inner code\" would improve readability and simplify UT. Thoughts?\n\n def reschedule_router(...):\n   for attempt in range(5):\n     try:\n       self._reschedule_router(...)\n       return\n     except:\n       LOG.warning(...)\n   else:\n     self._unbind_router(...)\n     raise ...","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"273c001c1ecbdf5c4dcb048f84c40d0b8bc7f8a0","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                    context, [router_id], new_agent.host)"},{"line_number":286,"context_line":"            except oslo_messaging.MessagingException:"},{"line_number":287,"context_line":"                max_attempts \u003d 5"},{"line_number":288,"context_line":"                LOG.warning(_LW(\u0027Failed to notify L3 agent on host %(host)s \u0027"},{"line_number":289,"context_line":"                                \u0027about added router. Attempt %(attempt)d out \u0027"},{"line_number":290,"context_line":"                                \u0027of %(max_attempts)d\u0027),"},{"line_number":291,"context_line":"                            {\u0027host\u0027: new_agent.host, \u0027attempt\u0027: attempt,"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa1b9901_4711adab","line":288,"in_reply_to":"fa1b9901_1fcc6b88","updated":"2015-08-18 06:53:15.000000000","message":"sure, I can refactor to a separate method. This however won\u0027t simplify unit tests much, do you think tests in the next module are too complicated?","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d023ec875ac2a74f2210c432501e983092aef617","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                    context, [router_id], new_agent.host)"},{"line_number":286,"context_line":"            except oslo_messaging.MessagingException:"},{"line_number":287,"context_line":"                max_attempts \u003d 5"},{"line_number":288,"context_line":"                LOG.warning(_LW(\u0027Failed to notify L3 agent on host %(host)s \u0027"},{"line_number":289,"context_line":"                                \u0027about added router. Attempt %(attempt)d out \u0027"},{"line_number":290,"context_line":"                                \u0027of %(max_attempts)d\u0027),"},{"line_number":291,"context_line":"                            {\u0027host\u0027: new_agent.host, \u0027attempt\u0027: attempt,"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa1b9901_eb3444df","line":288,"in_reply_to":"fa1b9901_4711adab","updated":"2015-08-18 10:16:56.000000000","message":"the main point is more readability, i am not sure it simplifies unittests but it could if the method becomes more complex","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"824476361fcaae660b306774e49a7ae5c807557c","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                            {\u0027host\u0027: new_agent.host, \u0027attempt\u0027: attempt,"},{"line_number":292,"context_line":"                             \u0027max_attempts\u0027: max_attempts})"},{"line_number":293,"context_line":"                if attempt \u003e\u003d max_attempts:"},{"line_number":294,"context_line":"                    self._unbind_router(context, router_id, new_agent[\u0027id\u0027])"},{"line_number":295,"context_line":"                    raise l3agentscheduler.RouterReschedulingFailed("},{"line_number":296,"context_line":"                        router_id\u003drouter_id)"},{"line_number":297,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a4dcd0f_37f82c7c","line":294,"updated":"2015-08-16 17:46:56.000000000","message":"What if _unbind_router fails?","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"9175473c16e68098ee4969019736b4c5181df31c","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                            {\u0027host\u0027: new_agent.host, \u0027attempt\u0027: attempt,"},{"line_number":292,"context_line":"                             \u0027max_attempts\u0027: max_attempts})"},{"line_number":293,"context_line":"                if attempt \u003e\u003d max_attempts:"},{"line_number":294,"context_line":"                    self._unbind_router(context, router_id, new_agent[\u0027id\u0027])"},{"line_number":295,"context_line":"                    raise l3agentscheduler.RouterReschedulingFailed("},{"line_number":296,"context_line":"                        router_id\u003drouter_id)"},{"line_number":297,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a4dcd0f_f7764405","line":294,"in_reply_to":"1a4dcd0f_37f82c7c","updated":"2015-08-16 17:56:06.000000000","message":"Plz ignore above comment. I didn\u0027t saw \"_unbind_router\" code correctly.","commit_id":"42a57669d299546a026f699f2861514c9460d317"},{"author":{"_account_id":16053,"name":"Pengtao Huang","email":"huangpengtao@huawei.com","username":"huangpengtao"},"change_message_id":"d97a7040c94ed82193bb60be57312e8b83ed0904","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            # Need to make sure agents are notified or unschedule otherwise"},{"line_number":309,"context_line":"            for attempt in range(AGENT_NOTIFY_MAX_ATTEMPTS):"},{"line_number":310,"context_line":"                try:"},{"line_number":311,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":312,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":313,"context_line":"                    break"},{"line_number":314,"context_line":"                except oslo_messaging.MessagingException:"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_83b09dfe","line":311,"updated":"2015-08-26 07:39:52.000000000","message":"you can change router_added_to_agent to use call,reference to https://review.openstack.org/#/c/210378/8/neutron/api/rpc/agentnotifiers/l3_rpc_agent_api.py","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b99d0e9557902eb080a9b3482a461f8570a936b0","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            # Need to make sure agents are notified or unschedule otherwise"},{"line_number":309,"context_line":"            for attempt in range(AGENT_NOTIFY_MAX_ATTEMPTS):"},{"line_number":310,"context_line":"                try:"},{"line_number":311,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":312,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":313,"context_line":"                    break"},{"line_number":314,"context_line":"                except oslo_messaging.MessagingException:"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_464053a5","line":311,"in_reply_to":"fa1b9901_26e19f2a","updated":"2015-08-26 08:06:53.000000000","message":"It won\u0027t retry many times if first attempt is successful. If not, then I think it is worth retrying to be more robust in case of intermittent messaging system failures","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":16053,"name":"Pengtao Huang","email":"huangpengtao@huawei.com","username":"huangpengtao"},"change_message_id":"475287f2918e5acbceb8d0fb0bfe290246b25616","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            # Need to make sure agents are notified or unschedule otherwise"},{"line_number":309,"context_line":"            for attempt in range(AGENT_NOTIFY_MAX_ATTEMPTS):"},{"line_number":310,"context_line":"                try:"},{"line_number":311,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":312,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":313,"context_line":"                    break"},{"line_number":314,"context_line":"                except oslo_messaging.MessagingException:"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_26e19f2a","line":311,"in_reply_to":"fa1b9901_43cb6553","updated":"2015-08-26 07:59:49.000000000","message":"sorry,then do you think it is need to retry many times.\nis it enough to use call?\nor if agents are notified error,maybe it don\u0027t just raise MessagingException","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"8b038491331875851a5ee37815e2535f0a793355","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            # Need to make sure agents are notified or unschedule otherwise"},{"line_number":309,"context_line":"            for attempt in range(AGENT_NOTIFY_MAX_ATTEMPTS):"},{"line_number":310,"context_line":"                try:"},{"line_number":311,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":312,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":313,"context_line":"                    break"},{"line_number":314,"context_line":"                except oslo_messaging.MessagingException:"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_43cb6553","line":311,"in_reply_to":"fa1b9901_83b09dfe","updated":"2015-08-26 07:44:30.000000000","message":"not sure what you\u0027re talking about, it already uses call","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":16053,"name":"Pengtao Huang","email":"huangpengtao@huawei.com","username":"huangpengtao"},"change_message_id":"f924cb7efe4b9041d2e6e36c095773ac9fee3e73","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                try:"},{"line_number":311,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":312,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":313,"context_line":"                    break"},{"line_number":314,"context_line":"                except oslo_messaging.MessagingException:"},{"line_number":315,"context_line":"                    LOG.warning(_LW(\u0027Failed to notify L3 agent on host \u0027"},{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_d005fdc1","line":313,"updated":"2015-08-26 07:33:11.000000000","message":"the logical is strange.Do you think it is need to retry many  times? how do you make sure agents are notified. router_added_to_agent will send message using cast","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e438800c8c025c76fbd960c3ae2d14141e916cbd","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                try:"},{"line_number":311,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":312,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":313,"context_line":"                    break"},{"line_number":314,"context_line":"                except oslo_messaging.MessagingException:"},{"line_number":315,"context_line":"                    LOG.warning(_LW(\u0027Failed to notify L3 agent on host \u0027"},{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_631ea90d","line":313,"in_reply_to":"fa1b9901_d005fdc1","updated":"2015-08-26 07:41:47.000000000","message":"it will use call, please see changes in l3_rpc_agent_api.py","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c6b4216739322dda898b67ee4e90db6c0ab15877","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                    LOG.warning(_LW(\u0027Failed to notify L3 agent on host \u0027"},{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":317,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"},{"line_number":318,"context_line":"                                {\u0027host\u0027: agent[\u0027host\u0027], \u0027attempt\u0027: attempt,"},{"line_number":319,"context_line":"                                 \u0027max_attempts\u0027: AGENT_NOTIFY_MAX_ATTEMPTS})"},{"line_number":320,"context_line":"            else:"},{"line_number":321,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_d6fd11b2","line":318,"updated":"2015-08-25 18:07:29.000000000","message":"nit: range(5) is [0, 1, 2, 3, 4] so this should be attempt+1 ?  Or just print \u0027retrying\u0027 ?","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e438800c8c025c76fbd960c3ae2d14141e916cbd","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                    LOG.warning(_LW(\u0027Failed to notify L3 agent on host \u0027"},{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":317,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"},{"line_number":318,"context_line":"                                {\u0027host\u0027: agent[\u0027host\u0027], \u0027attempt\u0027: attempt,"},{"line_number":319,"context_line":"                                 \u0027max_attempts\u0027: AGENT_NOTIFY_MAX_ATTEMPTS})"},{"line_number":320,"context_line":"            else:"},{"line_number":321,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_23999179","line":318,"in_reply_to":"fa1b9901_d6fd11b2","updated":"2015-08-26 07:41:47.000000000","message":"yeah, it should be \u0027attempt+1\u0027, \u0027retrying\u0027 is not quite accurate as we will not retry after the last attempt","commit_id":"819bfb54aba3ac038f708e27e9b5d4d1a06cfd09"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"709fe79607121791fc2014a09c515bdd986fc409","unresolved":false,"context_lines":[{"line_number":306,"context_line":""},{"line_number":307,"context_line":"        for agent in new_agents:"},{"line_number":308,"context_line":"            # Need to make sure agents are notified or unschedule otherwise"},{"line_number":309,"context_line":"            for attempt in range(AGENT_NOTIFY_MAX_ATTEMPTS):"},{"line_number":310,"context_line":"                try:"},{"line_number":311,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":312,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_0cb69adc","line":309,"updated":"2015-08-26 20:41:00.000000000","message":"The other way to fix the range() issue would have been range(1, AGENT_NOTIFY_MAX_ATTEMPTS + 1), don\u0027t know why I didn\u0027t think of that yesterday.","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"709fe79607121791fc2014a09c515bdd986fc409","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":317,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"},{"line_number":318,"context_line":"                                {\u0027host\u0027: agent[\u0027host\u0027], \u0027attempt\u0027: attempt + 1,"},{"line_number":319,"context_line":"                                 \u0027max_attempts\u0027: AGENT_NOTIFY_MAX_ATTEMPTS})"},{"line_number":320,"context_line":"            else:"},{"line_number":321,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"},{"line_number":322,"context_line":"                raise l3agentscheduler.RouterReschedulingFailed("}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_6c73964e","line":319,"updated":"2015-08-26 20:41:00.000000000","message":"So just a question on this.  Have you seen this fail?  It\u0027s just that without a sleep of some kind in here does sending 5 notifications back-to-back increase the chance of success?  Should you at least do a sleep(0) to yield?","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"ea87db6aec6bebd490960d591bab9130a254304e","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":317,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"},{"line_number":318,"context_line":"                                {\u0027host\u0027: agent[\u0027host\u0027], \u0027attempt\u0027: attempt + 1,"},{"line_number":319,"context_line":"                                 \u0027max_attempts\u0027: AGENT_NOTIFY_MAX_ATTEMPTS})"},{"line_number":320,"context_line":"            else:"},{"line_number":321,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"},{"line_number":322,"context_line":"                raise l3agentscheduler.RouterReschedulingFailed("}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_af6c5bd0","line":319,"in_reply_to":"da20952f_18b2e106","updated":"2015-09-01 10:38:57.000000000","message":"The main usage of reschedule_router() is from reschedule_routers_from_down_agents which itself is a looping task executed in a separate green thread.\n\nThe only other usage of reschedule_router() is from update_router() which is part of REST API: in case router external network is changed we need to check if current agent can handle new external network or reschedule router to a proper one. Currently if notification to a new agent is lost, nobody is aware of that. User gets success response, server thinks everything is ok, while in fact router just stops functioning. With the current fix user will have to wait only in case there are problems with messaging. I believe it\u0027s quite a rare case when external network of a router is changed and even more rare when it happens during messaging system instability. Reworking API call(s?) to make it (them) doing agent notifications asynchronously seems like a non-trivial task and I\u0027m not sure it\u0027s well justified in this case. Please let me know what you think.","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"5e08bb0ef209b7aa5bd30b98099438e888f44878","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":317,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"},{"line_number":318,"context_line":"                                {\u0027host\u0027: agent[\u0027host\u0027], \u0027attempt\u0027: attempt + 1,"},{"line_number":319,"context_line":"                                 \u0027max_attempts\u0027: AGENT_NOTIFY_MAX_ATTEMPTS})"},{"line_number":320,"context_line":"            else:"},{"line_number":321,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"},{"line_number":322,"context_line":"                raise l3agentscheduler.RouterReschedulingFailed("}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_ef7461fe","line":319,"in_reply_to":"da20952f_6c73964e","updated":"2015-08-27 08:29:41.000000000","message":"Yeah, I\u0027ve seen the case when messages were lost when using cast(), when messaging system was unstable (Rabbit cluster failover). When switched to call() I saw the failure on the first attempt and success on the second, here added 5 retries just to be on the safe side, but now thinking 5 might take too much time in case agent indeed goes offline (not a messaging system failure). I think 3 attempts should be enough.\n\nNot sure about sleep(0), I think rpc call itself yields already.","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"717755df57a2b8c67d0caf4c8195f8278c5859a1","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":317,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"},{"line_number":318,"context_line":"                                {\u0027host\u0027: agent[\u0027host\u0027], \u0027attempt\u0027: attempt + 1,"},{"line_number":319,"context_line":"                                 \u0027max_attempts\u0027: AGENT_NOTIFY_MAX_ATTEMPTS})"},{"line_number":320,"context_line":"            else:"},{"line_number":321,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"},{"line_number":322,"context_line":"                raise l3agentscheduler.RouterReschedulingFailed("}],"source_content_type":"text/x-python","patch_set":9,"id":"ba15a1d1_e262feac","line":319,"in_reply_to":"da20952f_af6c5bd0","updated":"2015-09-15 17:13:05.000000000","message":"The alternative is something like returning 202 Accepted from the REST API indicating the request was accepted for processing, but not necessarily successful.  Then, at least based on the current API, you could use the \"status\" field to reflect the state and give the user a way to know if something went wrong.  You could set the state to \"scheduling\" and it would just be stuck there if an agent didn\u0027t pick it up as expected.  The agent would set it to \"active\" when everything is good.  Ideally something would also set a router\u0027s status to \"error\" if it\u0027s clearly broken.\n\nAnyway, you\u0027re right that the delays should only be when stuff is broken, and it\u0027s not a huge deal.  I don\u0027t want to block over it.","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"ec4e5dbda338ab20f397f5ab5cebd52d8c591deb","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":317,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"},{"line_number":318,"context_line":"                                {\u0027host\u0027: agent[\u0027host\u0027], \u0027attempt\u0027: attempt + 1,"},{"line_number":319,"context_line":"                                 \u0027max_attempts\u0027: AGENT_NOTIFY_MAX_ATTEMPTS})"},{"line_number":320,"context_line":"            else:"},{"line_number":321,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"},{"line_number":322,"context_line":"                raise l3agentscheduler.RouterReschedulingFailed("}],"source_content_type":"text/x-python","patch_set":9,"id":"da20952f_18b2e106","line":319,"in_reply_to":"da20952f_ef7461fe","updated":"2015-08-28 18:47:17.000000000","message":"By default, a single failure is going to take 60 seconds or so (whatever the rpc response timeout is set to, and 60 is the default).  So, a sleep isn\u0027t important here.\n\nIs it OK for this code to block for 5 minutes (or 3) waiting on a successful response?  Isn\u0027t this going to block responding to a REST API call?  If so, I don\u0027t think that kind of wait is really acceptable.  This kind of thing really needs to be done asynchronously from the API.","commit_id":"506136ba7463d509a4e6e45a3d1c346307031d94"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"16ea78beb5cab588016230c8af017a04270c45ea","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# default messaging timeout is 60 sec, so 2 here is chosen to not block API"},{"line_number":58,"context_line":"# call for more than 2 minutes"},{"line_number":59,"context_line":"AGENT_NOTIFY_MAX_ATTEMPTS \u003d 2"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class RouterL3AgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_4c4c92c8","line":59,"updated":"2015-09-08 14:39:50.000000000","message":"The usual distributed pattern is that the central component (which can easily be a scaling bottleneck) sends only messages in a non-blocking way. All the blocking calls are done from the agents, because their blocking does not have system-wide consequences. While blocking in the central component can easily have system-wide effects. (Even if we block in a thread, that thread eats up scarcer resources on the controllers than on a host running an agent.)\n\nSo if this 2 minute blocking can happen in the server, that at some point may become a performance/scaling problem.","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"8bd4b5f70e3863376c952e5cda9d4fa6e3715a07","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# default messaging timeout is 60 sec, so 2 here is chosen to not block API"},{"line_number":58,"context_line":"# call for more than 2 minutes"},{"line_number":59,"context_line":"AGENT_NOTIFY_MAX_ATTEMPTS \u003d 2"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class RouterL3AgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_7bbfc9aa","line":59,"in_reply_to":"da20952f_4c4c92c8","updated":"2015-09-08 16:18:34.000000000","message":"I know this solution is far from ideal, but we need to somehow deal with the bug which was seen on real environments. I\u0027m open to any alternatives. Please also see comments in patch set 9.\n\nAs a side note: neutron servers can scale to some degree, so you can have more than 1 server in a deployment. Also 60 sec timeout seems too much to me, a system on which message delivery takes 60 seconds is.. umm.. questionable, isn\u0027t it? I might be missing something but I think this should be decreased to 5 seconds. Will look more into messaging stuff.","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"0ffe0069709182327348d1e110b743307af8cd4d","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# default messaging timeout is 60 sec, so 2 here is chosen to not block API"},{"line_number":58,"context_line":"# call for more than 2 minutes"},{"line_number":59,"context_line":"AGENT_NOTIFY_MAX_ATTEMPTS \u003d 2"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class RouterL3AgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_985abf29","line":59,"in_reply_to":"da20952f_7bbfc9aa","updated":"2015-09-09 10:54:23.000000000","message":"Scaling neutron-server is possible of course, but still harder than scaling neutron agents. First, concurrent neutron-server may be impossible with some plugins. Second, they still regularly block each other in DB transactions.\n\nPlease see a possible alternative in my other comment.\n\nRegarding one of your comments in patch set 9: call is *not* more reliable than cast. If a single message is lost  with probability p (let\u0027s say p\u003d0.01%), then if you send two messages for call(), then you have a higher probability that at least one of the two messages will get lost. Also if the server or the agent has some chance of going down while one message is en route, then this probability will also be higher for a request-response.\n\nSo call() has a nicely informative outcome from the client perspective when everything is flawless. But on the other hand it has more diverse, more complicated and more frequent fault modes than cast from the overall system perspective.","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":7805,"name":"Li Ma","email":"skywalker.nick@gmail.com","username":"Nick"},"change_message_id":"66e40b5bf64499566bb14ef25ef1cb1a04b45e52","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# default messaging timeout is 60 sec, so 2 here is chosen to not block API"},{"line_number":58,"context_line":"# call for more than 2 minutes"},{"line_number":59,"context_line":"AGENT_NOTIFY_MAX_ATTEMPTS \u003d 2"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class RouterL3AgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_3165ad1c","line":59,"in_reply_to":"da20952f_985abf29","updated":"2015-09-10 04:09:14.000000000","message":"It seems that we should identify why call() is available for use from messaging system if it doesn\u0027t make any sense? If messaging, mostly RabbitMQ is not reliable, why we continue to use it?\n\nWhat\u0027s wrong with the buggy messaging system? Can we work with oslo.messaging to get rid of buggy stuff?\n\nWhat we discuss here is far away from the topic. It is related with the overall software architecture. For this given context, I suggest we assume MQ is flawless and it is able to deliver message to the destination, call() is good for client-side application with more information it carries.\n\nIf there\u0027s anything wrong with RPC, raise some detailed tech talk with oslo team?\n\nFor this parameter, I suggest to make it as an option in the conf file. For production deployment, we always adjust rpc-timeout. Usually it is less than 60 sec.","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":8976,"name":"shihanzhang","email":"ayshihanzhang@gmail.com","username":"shihanzhang"},"change_message_id":"ce234bee070a9ea603861559b37343fab321259e","unresolved":false,"context_lines":[{"line_number":281,"context_line":"        cur_agents \u003d self.list_l3_agents_hosting_router("},{"line_number":282,"context_line":"            context, router_id)[\u0027agents\u0027]"},{"line_number":283,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":284,"context_line":"            for agent in cur_agents:"},{"line_number":285,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"            self.schedule_router(context, router_id, candidates\u003dcandidates)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da20952f_dc98d477","line":284,"updated":"2015-09-11 07:24:01.000000000","message":"I have a question, if router is a DVR router, do we also need to do _unbind_router?","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"47b3167391417d691379088918b73ed660728e75","unresolved":false,"context_lines":[{"line_number":281,"context_line":"        cur_agents \u003d self.list_l3_agents_hosting_router("},{"line_number":282,"context_line":"            context, router_id)[\u0027agents\u0027]"},{"line_number":283,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":284,"context_line":"            for agent in cur_agents:"},{"line_number":285,"context_line":"                self._unbind_router(context, router_id, agent[\u0027id\u0027])"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"            self.schedule_router(context, router_id, candidates\u003dcandidates)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba15a1d1_99e862e4","line":284,"in_reply_to":"da20952f_dc98d477","updated":"2015-09-15 14:28:54.000000000","message":"In order to reschedule we need to unbind first, so, yes","commit_id":"0a3ee2b79d8f5d65bfc8423caa600862442d4e68"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"389892425e77a4b00e9074b58c5ca3979c1b5349","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# default messaging timeout is 60 sec, so 2 here is chosen to not block API"},{"line_number":59,"context_line":"# call for more than 2 minutes"},{"line_number":60,"context_line":"AGENT_NOTIFY_MAX_ATTEMPTS \u003d 2"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class RouterL3AgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a2ca52d_ecb0cbdf","line":60,"updated":"2015-10-14 13:55:14.000000000","message":"Do we want to make this a config knob?","commit_id":"afd53dafaeb17960998ef24be54393a77d891e29"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c88ab4bc3af24cd6fa63f917aeb7d65a20f253ec","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# default messaging timeout is 60 sec, so 2 here is chosen to not block API"},{"line_number":59,"context_line":"# call for more than 2 minutes"},{"line_number":60,"context_line":"AGENT_NOTIFY_MAX_ATTEMPTS \u003d 2"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class RouterL3AgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a29b11f_112c9097","line":60,"in_reply_to":"5a2ca52d_ecb0cbdf","updated":"2015-10-23 09:12:17.000000000","message":"I don\u0027t think we should bother the admins with this","commit_id":"afd53dafaeb17960998ef24be54393a77d891e29"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"52e0388b435cc72a713ca44521da08f5fe1e29b3","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        if not l3_notifier:"},{"line_number":302,"context_line":"            return"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        old_hosts \u003d [agent[\u0027host\u0027] for agent in old_agents]"},{"line_number":305,"context_line":"        new_hosts \u003d [agent[\u0027host\u0027] for agent in new_agents]"},{"line_number":306,"context_line":"        for host in set(old_hosts) - set(new_hosts):"},{"line_number":307,"context_line":"            l3_notifier.router_removed_from_agent("}],"source_content_type":"text/x-python","patch_set":14,"id":"ba8a016a_0fb6baf8","line":304,"updated":"2015-11-19 23:53:47.000000000","message":"nit:  Why not just create a set to begin with?\n\nold_hosts \u003d {agent[\u0027host\u0027] for agent in old_agents}\n\nupdate: I see you\u0027re moving this code from another place.  Ok to keep it like this if you want.","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"1d601d3bcebb28cc455a41aa857de2e52fd1e2c5","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":315,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":316,"context_line":"                    break"},{"line_number":317,"context_line":"                except oslo_messaging.MessagingException:"},{"line_number":318,"context_line":"                    LOG.warning(_LW(\u0027Failed to notify L3 agent on host \u0027"},{"line_number":319,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":320,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_333ad4e8","line":317,"updated":"2015-11-13 11:56:31.000000000","message":"Should we not be catching a timeout exception here? What if the agent is under load?","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"4c04a8663e94da2e10fa0a99c2b28fa35a886e73","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":315,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":316,"context_line":"                    break"},{"line_number":317,"context_line":"                except oslo_messaging.MessagingException:"},{"line_number":318,"context_line":"                    LOG.warning(_LW(\u0027Failed to notify L3 agent on host \u0027"},{"line_number":319,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":320,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"}],"source_content_type":"text/x-python","patch_set":14,"id":"da85f559_d668168a","line":317,"in_reply_to":"da85f559_333ad4e8","updated":"2015-11-13 12:02:59.000000000","message":"Not sure I got your comment. MessagingTimeout is derived from MessagingException so we should be on the safe side.","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d83c67a95540558e7b5b66191b03ae201a170dbf","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                    l3_notifier.router_added_to_agent("},{"line_number":315,"context_line":"                        context, [router_id], agent[\u0027host\u0027])"},{"line_number":316,"context_line":"                    break"},{"line_number":317,"context_line":"                except oslo_messaging.MessagingException:"},{"line_number":318,"context_line":"                    LOG.warning(_LW(\u0027Failed to notify L3 agent on host \u0027"},{"line_number":319,"context_line":"                                    \u0027%(host)s about added router. Attempt \u0027"},{"line_number":320,"context_line":"                                    \u0027%(attempt)d out of %(max_attempts)d\u0027),"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba8a016a_1c572827","line":317,"in_reply_to":"da85f559_d668168a","updated":"2015-11-18 17:16:23.000000000","message":"understood","commit_id":"30b121dfa4d8542665b2209d21ece805d8957aa5"}],"neutron/tests/unit/db/test_agentschedulers_db.py":[{"author":{"_account_id":7715,"name":"Hirofumi Ichihara","email":"ichihara.hirofumi@gmail.com","username":"Hirofumi-Ichihara"},"change_message_id":"78a25a21ae9a4abba6edaa50f287289717e226a4","unresolved":false,"context_lines":[{"line_number":815,"context_line":"                    l3_notifier.router_added_to_agent.call_count)"},{"line_number":816,"context_line":"                l3_agents \u003d self._list_l3_agents_hosting_router("},{"line_number":817,"context_line":"                    router[\u0027router\u0027][\u0027id\u0027])[\u0027agents\u0027]"},{"line_number":818,"context_line":"                self.assertFalse(len(l3_agents))"},{"line_number":819,"context_line":""},{"line_number":820,"context_line":"    def test_router_auto_schedule_with_invalid_router(self):"},{"line_number":821,"context_line":"        with self.router() as router:"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a2ca52d_7ba0d390","line":818,"updated":"2015-10-15 07:10:07.000000000","message":"assertFalse shoud be passed boolean type. It\u0027s better to use self.assertEqual(0, len(l3_agents))","commit_id":"afd53dafaeb17960998ef24be54393a77d891e29"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c88ab4bc3af24cd6fa63f917aeb7d65a20f253ec","unresolved":false,"context_lines":[{"line_number":815,"context_line":"                    l3_notifier.router_added_to_agent.call_count)"},{"line_number":816,"context_line":"                l3_agents \u003d self._list_l3_agents_hosting_router("},{"line_number":817,"context_line":"                    router[\u0027router\u0027][\u0027id\u0027])[\u0027agents\u0027]"},{"line_number":818,"context_line":"                self.assertFalse(len(l3_agents))"},{"line_number":819,"context_line":""},{"line_number":820,"context_line":"    def test_router_auto_schedule_with_invalid_router(self):"},{"line_number":821,"context_line":"        with self.router() as router:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a29b11f_d121389f","line":818,"in_reply_to":"5a2ca52d_7ba0d390","updated":"2015-10-23 09:12:17.000000000","message":"Done","commit_id":"afd53dafaeb17960998ef24be54393a77d891e29"}]}