)]}'
{"neutron/scheduler/l3_agent_scheduler.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"0d7a4c42ceb473880fff32a992427d99df2351e0","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                        plugin, context, router[\u0027id\u0027],"},{"line_number":208,"context_line":"                        router[\u0027tenant_id\u0027], l3_agent)"},{"line_number":209,"context_line":"            else:"},{"line_number":210,"context_line":"                self.bind_router_legacy(context, router[\u0027id\u0027], l3_agent)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def bind_router(self, context, router_id, chosen_agent):"},{"line_number":213,"context_line":"        \"\"\"Bind the router to the l3 agent which has been chosen.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"1ac06dbe_84a5a0b6","line":210,"range":{"start_line":210,"start_character":21,"end_line":210,"end_character":39},"updated":"2016-08-22 12:21:45.000000000","message":"This code path is not only for legacy router, but also for dvr router.","commit_id":"f7927ed97893c1ff4873c20e860c786c9542bdcc"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"5d2003dbb22a96116f59a784821516c6ca1088ba","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                        plugin, context, router[\u0027id\u0027],"},{"line_number":208,"context_line":"                        router[\u0027tenant_id\u0027], l3_agent)"},{"line_number":209,"context_line":"            else:"},{"line_number":210,"context_line":"                self.bind_router_legacy(context, router[\u0027id\u0027], l3_agent)"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def bind_router(self, context, router_id, chosen_agent):"},{"line_number":213,"context_line":"        \"\"\"Bind the router to the l3 agent which has been chosen.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"1ac06dbe_a5b8c7e6","line":210,"range":{"start_line":210,"start_character":21,"end_line":210,"end_character":39},"in_reply_to":"1ac06dbe_84a5a0b6","updated":"2016-08-25 04:40:19.000000000","message":"I see. I will remove this code path in new PS and add a check before calling this method instead.","commit_id":"f7927ed97893c1ff4873c20e860c786c9542bdcc"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"0d7a4c42ceb473880fff32a992427d99df2351e0","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                chosen_agent_id \u003d chosen_agent[\u0027id\u0027]"},{"line_number":240,"context_line":"                existing_bindings \u003d self._check_router_existing_l3_agent("},{"line_number":241,"context_line":"                    context, router_id)"},{"line_number":242,"context_line":"                if existing_bindings:"},{"line_number":243,"context_line":"                    for existing_binding in existing_bindings:"},{"line_number":244,"context_line":"                        if existing_binding.l3_agent_id !\u003d chosen_agent_id:"},{"line_number":245,"context_line":"                            LOG.debug("}],"source_content_type":"text/x-python","patch_set":9,"id":"1ac06dbe_38f4cc84","line":242,"updated":"2016-08-22 12:21:45.000000000","message":"Is it possible to use  the new router status ROUTER_STATUS_ALLOCATING to implement the same thing? I think that will be cleaner in code.","commit_id":"f7927ed97893c1ff4873c20e860c786c9542bdcc"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"5d2003dbb22a96116f59a784821516c6ca1088ba","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                chosen_agent_id \u003d chosen_agent[\u0027id\u0027]"},{"line_number":240,"context_line":"                existing_bindings \u003d self._check_router_existing_l3_agent("},{"line_number":241,"context_line":"                    context, router_id)"},{"line_number":242,"context_line":"                if existing_bindings:"},{"line_number":243,"context_line":"                    for existing_binding in existing_bindings:"},{"line_number":244,"context_line":"                        if existing_binding.l3_agent_id !\u003d chosen_agent_id:"},{"line_number":245,"context_line":"                            LOG.debug("}],"source_content_type":"text/x-python","patch_set":9,"id":"1ac06dbe_65a52fb4","line":242,"in_reply_to":"1ac06dbe_38f4cc84","updated":"2016-08-25 04:40:19.000000000","message":"I do not think we can use ROUTER_STATUS_ALLOCATING here. It is used only by HA routers.","commit_id":"f7927ed97893c1ff4873c20e860c786c9542bdcc"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4cd4bd8232ae413f671f7c2cfab9350e89381d50","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                        plugin, context, router[\u0027id\u0027],"},{"line_number":197,"context_line":"                        router[\u0027tenant_id\u0027], l3_agent)"},{"line_number":198,"context_line":"            else:"},{"line_number":199,"context_line":"                existing_l3_agent \u003d plugin.get_l3_agents_hosting_routers("},{"line_number":200,"context_line":"                    context, [router[\u0027id\u0027]])"},{"line_number":201,"context_line":"                if existing_l3_agent:"},{"line_number":202,"context_line":"                    LOG.debug("}],"source_content_type":"text/x-python","patch_set":10,"id":"1ac06dbe_43d459bc","line":199,"range":{"start_line":199,"start_character":36,"end_line":199,"end_character":72},"updated":"2016-08-25 10:54:36.000000000","message":"This only changes the timings of the problem but doesn\u0027t eliminate it. Consider that 2 different workers are running this code at the same time. Previously, 2 workers were simply executing self.bind_router() at the same time regardless if it was already done. Now, with this code, 2 different workers might also run get_l3_agents_hosting_routers() at the exact same time, see that no scheduling has been done yet (since both are still in plugin.get_l3_agents_hosting_routers), and continue on to self.bind_router().\n\nThis patch merely lowers the chances of encountering the race, and doesn\u0027t eliminate it.","commit_id":"4c1375c7e4b6bb1917f7aafaa29c469db44bb2e2"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"f52198faf4de5fc11dc12a0c8235abd97450584a","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                        plugin, context, router[\u0027id\u0027],"},{"line_number":197,"context_line":"                        router[\u0027tenant_id\u0027], l3_agent)"},{"line_number":198,"context_line":"            else:"},{"line_number":199,"context_line":"                existing_l3_agent \u003d plugin.get_l3_agents_hosting_routers("},{"line_number":200,"context_line":"                    context, [router[\u0027id\u0027]])"},{"line_number":201,"context_line":"                if existing_l3_agent:"},{"line_number":202,"context_line":"                    LOG.debug("}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_599e5957","line":199,"range":{"start_line":199,"start_character":36,"end_line":199,"end_character":72},"in_reply_to":"1ac06dbe_43d459bc","updated":"2016-08-26 05:39:47.000000000","message":"Thanks for the review John, also the explanations here! I will try to find another approach to eliminate this race condition.","commit_id":"4c1375c7e4b6bb1917f7aafaa29c469db44bb2e2"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"16b5a843bcc30ce5698ad36b7451e532d37bc135","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                        plugin, context, router[\u0027id\u0027],"},{"line_number":197,"context_line":"                        router[\u0027tenant_id\u0027], l3_agent)"},{"line_number":198,"context_line":"            else:"},{"line_number":199,"context_line":"                existing_l3_agent \u003d plugin.get_l3_agents_hosting_routers("},{"line_number":200,"context_line":"                    context, [router[\u0027id\u0027]])"},{"line_number":201,"context_line":"                if existing_l3_agent:"},{"line_number":202,"context_line":"                    LOG.debug("}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_0632055f","line":199,"range":{"start_line":199,"start_character":36,"end_line":199,"end_character":72},"in_reply_to":"fa7ab95a_599e5957","updated":"2016-08-30 03:05:26.000000000","message":"Hi John, would you mind to check the latest patch if you have time? I have changed the approach to post clean-up. Your advice would be appreciated.","commit_id":"4c1375c7e4b6bb1917f7aafaa29c469db44bb2e2"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"ec13a0097e80918bf16ebdc8b77850549517465d","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            else:"},{"line_number":199,"context_line":"                existing_l3_agent \u003d plugin.get_l3_agents_hosting_routers("},{"line_number":200,"context_line":"                    context, [router[\u0027id\u0027]])"},{"line_number":201,"context_line":"                if existing_l3_agent:"},{"line_number":202,"context_line":"                    LOG.debug("},{"line_number":203,"context_line":"                        \u0027Router %(router_id)s has already been scheduled \u0027"},{"line_number":204,"context_line":"                        \u0027to L3 agent %(agent_id)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1ac06dbe_c38b2967","line":201,"updated":"2016-08-25 10:54:18.000000000","message":"This isn\u0027t atomic... This conditional can return False and then another process schedules the router, and then this process continues to line 208. The ideal way to solve this is via a DB constraint and catching a duplicate error. If that is not possible then you can do something similar to:\nhttps://review.openstack.org/#/c/282876","commit_id":"4c1375c7e4b6bb1917f7aafaa29c469db44bb2e2"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"16b5a843bcc30ce5698ad36b7451e532d37bc135","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            else:"},{"line_number":199,"context_line":"                existing_l3_agent \u003d plugin.get_l3_agents_hosting_routers("},{"line_number":200,"context_line":"                    context, [router[\u0027id\u0027]])"},{"line_number":201,"context_line":"                if existing_l3_agent:"},{"line_number":202,"context_line":"                    LOG.debug("},{"line_number":203,"context_line":"                        \u0027Router %(router_id)s has already been scheduled \u0027"},{"line_number":204,"context_line":"                        \u0027to L3 agent %(agent_id)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_46016d69","line":201,"in_reply_to":"1ac06dbe_c38b2967","updated":"2016-08-30 03:05:26.000000000","message":"Hi Assaf, after thinking about the link you left for two days, i do not think this patch can follow the concept in that link. The helper function in that link requires two steps. Using Kevin\u0027s ptach as an example, they would be: 1. create a HA network, 2. create HA network interface. The race it eliminated is in step 1. \n\nHowever in this patch set, the router and L3 agents to be bound have been picked out. Then bind them together. \n\nSince it is also hard to add DB constraint, the approach i can figure out is to do a post clean up. After calling bind_router(), I added a method called _unbind_excess_l3_agents() to check if the router is scheduled to multiple L3 agents. If so, I delete excess L3 agents in a deterministic way by sorting agent_id. This is to prevent multiple processes from deleting different L3 agents and leaving the router without any L3 agent.","commit_id":"4c1375c7e4b6bb1917f7aafaa29c469db44bb2e2"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"f52198faf4de5fc11dc12a0c8235abd97450584a","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            else:"},{"line_number":199,"context_line":"                existing_l3_agent \u003d plugin.get_l3_agents_hosting_routers("},{"line_number":200,"context_line":"                    context, [router[\u0027id\u0027]])"},{"line_number":201,"context_line":"                if existing_l3_agent:"},{"line_number":202,"context_line":"                    LOG.debug("},{"line_number":203,"context_line":"                        \u0027Router %(router_id)s has already been scheduled \u0027"},{"line_number":204,"context_line":"                        \u0027to L3 agent %(agent_id)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa7ab95a_99e4610e","line":201,"in_reply_to":"1ac06dbe_c38b2967","updated":"2016-08-26 05:39:47.000000000","message":"I find it hard to add a DB constraint here since the RouterL3AgentBinding table is used by both HA and non-HA routers. Non-HA routers only allow one L3 agent but HA routers are hosted by multiple L3 agents. \n\nI will look into the link you gave here. Thanks Assaf!","commit_id":"4c1375c7e4b6bb1917f7aafaa29c469db44bb2e2"}]}