)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"72db66a3418f6a279010b8e16e747bc1bac22d22","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure no stale process lives in l3 agent side after races"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As the patch https://review.openstack.org/#/c/265685/ mentioned"},{"line_number":10,"context_line":"that race may happen after a delete RPC between l3 agents."},{"line_number":11,"context_line":"The metadata-proxy process will be spawned and alive"},{"line_number":12,"context_line":"after all info of a HA router was deleted in the l3 agent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da6ed579_b1a79f4e","line":9,"updated":"2016-01-12 14:52:38.000000000","message":"The race is not clear to me. What are the two parties that are racing? Can you give an example?","commit_id":"3beaee635a489d413ad3e3c1d357f266fc9df2db"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"380a05a9feeb95069ce8c8c580e00566e4a74757","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure no stale process lives in l3 agent side after races"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As the patch https://review.openstack.org/#/c/265685/ mentioned"},{"line_number":10,"context_line":"that race may happen after a delete RPC between l3 agents."},{"line_number":11,"context_line":"The metadata-proxy process will be spawned and alive"},{"line_number":12,"context_line":"after all info of a HA router was deleted in the l3 agent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da6ed579_1d5c5eec","line":9,"in_reply_to":"da6ed579_1aa9c4df","updated":"2016-01-13 02:32:19.000000000","message":"Sorry for the inaccurate：13. metadata-proxy for router X in agent B lives.\nThe only way to decide whether to spawn the metedata-proxy is to try get router in agent router_info dict. But enqueue_state_change and processing router deleting can be run concurrently.","commit_id":"3beaee635a489d413ad3e3c1d357f266fc9df2db"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"001997ccba0832f8e6d4f13aa4607826b47fc7fe","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ensure no stale process lives in l3 agent side after races"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As the patch https://review.openstack.org/#/c/265685/ mentioned"},{"line_number":10,"context_line":"that race may happen after a delete RPC between l3 agents."},{"line_number":11,"context_line":"The metadata-proxy process will be spawned and alive"},{"line_number":12,"context_line":"after all info of a HA router was deleted in the l3 agent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da6ed579_1aa9c4df","line":9,"in_reply_to":"da6ed579_b1a79f4e","updated":"2016-01-13 01:57:09.000000000","message":"The race happened between l3 agents after a fanout deleting HA router RPC. Race Scenario:\n1. HA router X was schedulered to L3 agent A and L3 agent B\n\n\n2. X in L3 agent A is the master state\n\n\n3. a delete X RPC fanout\n\n\n4. agent A delete all X HA attributes and processes including keepalived\n\n\n5. （race） agent B was not ready to process the deleting RPC, \nassume there are a lot of deleting RPC is in the router update \nqueue, or anything cause the agent B delay processing the RPC.\n\n\n6. (race) X in agent B is backup state, now it can not get the VRRP advertisement from X in agent A because of the 4, so X set it\u0027s state to master\n\n\n8. (race) enqueue_state_change for X in agent B\n\n\n9. (race) agent B could process the deleting RPC\n\n\n10. (race) X is still in agent B router_info, so spawn the metadata-proxy\n\n\n11. (race) agent B do deleting process for HA router X gateway, floating IP etc.\n\n\n12. (race) agent B remove X from router info\n\n\n13. metadata-proxy for router B lives\n\n\nIf you have tried to use rally to run create_and_delete_routers, you will find the l3 agent side will have some stale metadata-proxy processes after the rally test.","commit_id":"3beaee635a489d413ad3e3c1d357f266fc9df2db"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"928d0b64a9cd86f8289455869f30638dbb92e3d2","unresolved":false,"context_lines":[{"line_number":19,"context_line":"make the spwan condition just more specific."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I9b1eadc24b9dc6285ca47156f9ee0c9303c22dac"},{"line_number":22,"context_line":"Closes-Bug: #1523780"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da6ed579_978a74be","line":22,"range":{"start_line":22,"start_character":13,"end_line":22,"end_character":20},"updated":"2016-01-12 23:02:21.000000000","message":"I\u0027ve got a bunch of reviews that claim to close the same bug [1-7].  In general, we can only have one review close a bug.  If it needs other patches then it needs to depend on them and then wait for them all to get merged and then close the bug.\n\nAs it is now, the first one of these to merge will close the bug and then we\u0027ll be reviewing the rest on a bug marked closed.  Something needs to be done.  Here are some options.\n\n1.  You could line them all up properly with most of them marked with related-bug instead of closes-bug with the last one coming in a closing it.\n2.  You could keep them independent and file a bug for each independent fix.\n3.  Some mix of 1 and 2.\n4.  Tell me that they are all independent fixes for the same bug and which one is the best one to review (probably abandon most of the others).\n\n[1] https://review.openstack.org/#/c/265672/\n[2] https://review.openstack.org/#/c/265676/\n[3] https://review.openstack.org/#/c/265680/\n[4] https://review.openstack.org/#/c/265682/\n[5] https://review.openstack.org/#/c/265685/\n[6] https://review.openstack.org/#/c/266109/\n[7] https://review.openstack.org/#/c/260303/","commit_id":"3beaee635a489d413ad3e3c1d357f266fc9df2db"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"d9977a89421645e1bd69cc02938d5597af7754a5","unresolved":false,"context_lines":[{"line_number":19,"context_line":"make the spwan condition just more specific."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I9b1eadc24b9dc6285ca47156f9ee0c9303c22dac"},{"line_number":22,"context_line":"Closes-Bug: #1523780"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da6ed579_9316ffef","line":22,"range":{"start_line":22,"start_character":13,"end_line":22,"end_character":20},"in_reply_to":"da6ed579_46644848","updated":"2016-01-13 03:19:11.000000000","message":"New LP bug #1533455","commit_id":"3beaee635a489d413ad3e3c1d357f266fc9df2db"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"001997ccba0832f8e6d4f13aa4607826b47fc7fe","unresolved":false,"context_lines":[{"line_number":19,"context_line":"make the spwan condition just more specific."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I9b1eadc24b9dc6285ca47156f9ee0c9303c22dac"},{"line_number":22,"context_line":"Closes-Bug: #1523780"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da6ed579_46644848","line":22,"range":{"start_line":22,"start_character":13,"end_line":22,"end_character":20},"in_reply_to":"da6ed579_978a74be","updated":"2016-01-13 01:57:09.000000000","message":"Thank you, as Kevin Benton mentioned in Patch Set 10 https://review.openstack.org/#/c/260303/10:\n\"to elaborate, the commit message identifies 4 different race conditions. please fix one in each patch so it\u0027s clear what is being done\". Different people have different views, so now each patch deal with each one race.\nMaybe I should register a separated lp bug for each patch.","commit_id":"3beaee635a489d413ad3e3c1d357f266fc9df2db"}],"neutron/agent/l3/ha.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"612e5011ba3dee5636339a9729b5ac7bb940a959","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_6b4af427","line":224,"updated":"2016-01-25 06:43:08.000000000","message":"I don\u0027t think the code here can ultimately resolve the issue of concurrency. Here is multi-thread. The code execution can stop at line 225. And the router can be deleted in another thread during this stop. If that happens, your check at line 222 and 223 will be out of date.\n\nI don\u0027t think it is a easy way to solve the concurrency issue here, pls consider more.","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"887f4db2768cbd9f50aa2c55e92738697df4aaf2","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_8a1e0b5e","line":224,"in_reply_to":"9a68dd71_477b2e2a","updated":"2016-01-26 06:26:50.000000000","message":"\"_ensure_master_router_can_spawn_metadata_proxy\" has a comment \"To save the RPC call here check the HA router attributes first.\". Race between some HA router deleting procedure like \"ha_network_removed\" and keepalived_state_change \"notify_agent\".","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"d213fcdac2c745d616c210cee1fed40136e70aed","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_ea9d4cbc","line":224,"in_reply_to":"9a68dd71_4f17ca44","updated":"2016-01-26 02:23:17.000000000","message":"[1] will prevent race for router update in your description. \nI might want to say \"Due to the thread scheduling, code \u0027might\u0027 stop at line 225.\" It is small chance, but there is still a possibility.\n\nMy point here is that, your code here can\u0027t fully resolve the problem. But I do believe your code can mitigate the problem. You can\u0027t count on one thread to check and resolve the problem of threads synchronization. However, I don\u0027t have a good idea to resolve the problem of threads synchronization here. Or else, I will bring it out in my first comment.\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/agent/l3/router_processing_queue.py#L61","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"0c8af446b6946d7582baf11d6700b53ad84538e9","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_477b2e2a","line":224,"in_reply_to":"9a68dd71_5b61c803","updated":"2016-01-26 05:58:48.000000000","message":"what/why causes \"code \u0027might\u0027 stop at line 225.\" ?\nThe thread scheduling might cause this. I think you can find lots of explanation from internet. \n\nThink more, \"_is_router_still_existed_in_neutron_server\" might be helpful to avoid the case you mentioned. But what is the meaning of \"_ensure_master_router_can_spawn_metadata_proxy\" here? It will return true as long as agent don\u0027t get the delete rpc. Anyway, I will insist my -1 for this PS. But, that will not prevent others from +1/+2.","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ab84eb0152e9b42e585cc08eb37427c4f4900f32","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_fe4a2054","line":224,"in_reply_to":"9a68dd71_6b4af427","updated":"2016-01-25 07:25:14.000000000","message":"Thank you. Here is to deal with the race between L3 agents after a HA router \u0027DELETE\u0027 RPC. As you said that the router can be deleted and code stop at 225, so there will have no more stale metadata-proxy process for each \u0027deleting\u0027 router. So could you please explain more?","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"edf83e6c5ad99f19f921f6f7f359c6bfb93deab5","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_4f17ca44","line":224,"in_reply_to":"9a68dd71_de315c5b","updated":"2016-01-26 01:44:09.000000000","message":"Just calm down, depending on your opinion there are race everywhere, for instance, the router update pool size is 8, router A update rpc-1 processed in one thread, and router A update rpc-2 processed in another thread, so which is the updating result ? And could you explain which mechanism causes \"Due to the thread scheduling, code stops at line 225.\" ? Or can you reproduce the scenario you described?","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"26b24020404c7ddbcf97266bdf32408fbd43d7fe","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_5b61c803","line":224,"in_reply_to":"9a68dd71_ea9d4cbc","updated":"2016-01-26 05:27:48.000000000","message":"Thank you very much for explain so much, sorry for the inappropriate example. But is there any mechanism causes python threads may stop at a \"if\" expression? So you still do not explain what/why causes \"code \u0027might\u0027 stop at line 225.\" ? And as the commit message says \"that race may happen after a delete RPC between l3 agents.\". And the state_changes are all because the \u0027master\u0027 router delete it\u0027s keepalived instance. And your described state_change may cause by something else. So the logic code here are all aim to deal with the state change that neutron server already done the DELETE rpc, this means router\u0027s all stuffs are deleted including HA attributes. Line 223 can make sure that HA router whether still exist in neutron server side, if not, Line 255 will get a False.\n\n\nMaybe replace your description \"2) Agent here still don\u0027t get the delete rpc.\" with \"2) neutron server still not execute the delete procedure.\" will looks more proper, and you can submit a new LP bug to deal with that small chance scenario.","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"8e186ba8206960e4768334271e88d4d2f547c418","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":222,"context_line":"                and self._ensure_master_router_can_spawn_metadata_proxy(ri)"},{"line_number":223,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":224,"context_line":"                    ri, \u0027spawn metadata proxy\u0027)):"},{"line_number":225,"context_line":"            # Ensure that all the HA router resources/attributes are correct"},{"line_number":226,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":227,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_de315c5b","line":224,"in_reply_to":"9a68dd71_fe4a2054","updated":"2016-01-25 07:43:10.000000000","message":"The procedure in my mind is like this:\n1) Ha router change to \u0027master\u0027 state.\n2) Agent here still don\u0027t get the delete rpc.\n3) Code in enqueue_state_change is being executed, and hit line 225. Because the agent still don\u0027t get the delete rpc, the router is still in the agent. So, the metadata process is to be created.\n4) Due to the thread scheduling, code stops at line 225.\n5) At this time, the delete rpc comes, and agent delete router. \n6) The metadata process becomes orphan.","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"612e5011ba3dee5636339a9729b5ac7bb940a959","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        # Radvd has to be spawned only on the Master HA Router. If there are"},{"line_number":237,"context_line":"        # any state transitions, we enable/disable radvd accordingly."},{"line_number":238,"context_line":"        if (state \u003d\u003d \u0027master\u0027"},{"line_number":239,"context_line":"                and self._is_router_still_existed_in_neutron_server("},{"line_number":240,"context_line":"                    ri, \u0027enable radvd\u0027)):"},{"line_number":241,"context_line":"            ri.enable_radvd()"},{"line_number":242,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_eb64a4b9","line":239,"updated":"2016-01-25 06:43:08.000000000","message":"See my comments above.","commit_id":"0a633f224128c43b2844d022c6bd14783bf0df53"}]}