)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0037c7fd7f06acfe5dfbe028f0be00df333bffdb","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Kailun Qin \u003ckailun.qin@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-06 19:25:10 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Synchronize agent scheduling to prevent the race"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"A race condition may occur when concurrent agent scheduling happens,"},{"line_number":10,"context_line":"since neutron-server uses multiple child processes to handle the agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5f7c97a3_5735b0d9","line":7,"range":{"start_line":7,"start_character":40,"end_line":7,"end_character":43},"updated":"2018-07-09 20:04:50.000000000","message":"nit: a","commit_id":"72a2862d275be0d76d8db7f5916dddbdaeb3b26b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0037c7fd7f06acfe5dfbe028f0be00df333bffdb","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Closes-Bug: #1780357"},{"line_number":18,"context_line":"Change-Id: If1fac6c2986fb8a0a5a3c0f6526bd0ceaba86d36"},{"line_number":19,"context_line":"Signed-off-by: Kailun Qin \u003ckailun.qin@intel.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5f7c97a3_573a90c3","line":19,"updated":"2018-07-09 20:04:50.000000000","message":"We don\u0027t require a signoff in neutron","commit_id":"72a2862d275be0d76d8db7f5916dddbdaeb3b26b"}],"neutron/db/agentschedulers_db.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0037c7fd7f06acfe5dfbe028f0be00df333bffdb","unresolved":false,"context_lines":[{"line_number":472,"context_line":"    @runtime.synchronized(\u0027schedule-network\u0027, external\u003dTrue)"},{"line_number":473,"context_line":"    def schedule_network(self, context, created_network):"},{"line_number":474,"context_line":"        if self.network_scheduler:"},{"line_number":475,"context_line":"            return self.network_scheduler.schedule("},{"line_number":476,"context_line":"                self, context, created_network)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    @runtime.synchronized(\u0027auto-schedule-network\u0027, external\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_978108fd","line":475,"updated":"2018-07-09 20:04:50.000000000","message":"Is this still atomic as auto_schedule_routers() will call self.schedule(), which won\u0027t acquire this lock.","commit_id":"72a2862d275be0d76d8db7f5916dddbdaeb3b26b"},{"author":{"_account_id":28373,"name":"Kailun Qin","email":"kailun.qin@hotmail.com","username":"kailun.qin"},"change_message_id":"8f8fe7ec62fba3cf3f4312c24ee54abd674818d1","unresolved":false,"context_lines":[{"line_number":472,"context_line":"    @runtime.synchronized(\u0027schedule-network\u0027, external\u003dTrue)"},{"line_number":473,"context_line":"    def schedule_network(self, context, created_network):"},{"line_number":474,"context_line":"        if self.network_scheduler:"},{"line_number":475,"context_line":"            return self.network_scheduler.schedule("},{"line_number":476,"context_line":"                self, context, created_network)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    @runtime.synchronized(\u0027auto-schedule-network\u0027, external\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_2676c0d6","line":475,"in_reply_to":"5f7c97a3_978108fd","updated":"2018-07-10 10:33:37.000000000","message":"It should still be atomic as the two schedules are not the same thing.\nHere what is calling is the schedule() method of BaseScheduler which is inherited only in dhcp_agent_scheduler. However, self.schedule() in auto_schedule_routers() calls the method of L3Scheduler which is already a top most parent and not related with BaseScheduler.\nNot sure if I understand the concern right. Just correct me if I\u0027m wrong.","commit_id":"72a2862d275be0d76d8db7f5916dddbdaeb3b26b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0037c7fd7f06acfe5dfbe028f0be00df333bffdb","unresolved":false,"context_lines":[{"line_number":475,"context_line":"            return self.network_scheduler.schedule("},{"line_number":476,"context_line":"                self, context, created_network)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    @runtime.synchronized(\u0027auto-schedule-network\u0027, external\u003dTrue)"},{"line_number":479,"context_line":"    def auto_schedule_networks(self, context, host):"},{"line_number":480,"context_line":"        if self.network_scheduler:"},{"line_number":481,"context_line":"            self.network_scheduler.auto_schedule_networks(self, context, host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_9706e81a","line":478,"range":{"start_line":478,"start_character":41,"end_line":478,"end_character":48},"updated":"2018-07-09 20:04:50.000000000","message":"s/networks","commit_id":"72a2862d275be0d76d8db7f5916dddbdaeb3b26b"},{"author":{"_account_id":28373,"name":"Kailun Qin","email":"kailun.qin@hotmail.com","username":"kailun.qin"},"change_message_id":"8f8fe7ec62fba3cf3f4312c24ee54abd674818d1","unresolved":false,"context_lines":[{"line_number":475,"context_line":"            return self.network_scheduler.schedule("},{"line_number":476,"context_line":"                self, context, created_network)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    @runtime.synchronized(\u0027auto-schedule-network\u0027, external\u003dTrue)"},{"line_number":479,"context_line":"    def auto_schedule_networks(self, context, host):"},{"line_number":480,"context_line":"        if self.network_scheduler:"},{"line_number":481,"context_line":"            self.network_scheduler.auto_schedule_networks(self, context, host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_e64d2899","line":478,"range":{"start_line":478,"start_character":41,"end_line":478,"end_character":48},"in_reply_to":"5f7c97a3_9706e81a","updated":"2018-07-10 10:33:37.000000000","message":"Accept. Thanks for pointing this out!","commit_id":"72a2862d275be0d76d8db7f5916dddbdaeb3b26b"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d3ac1751c936c0d1de902e962c4ab9cef952abde","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        else:"},{"line_number":470,"context_line":"            return {\u0027agents\u0027: []}"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    @runtime.synchronized(\u0027schedule-network\u0027, external\u003dTrue)"},{"line_number":473,"context_line":"    def schedule_network(self, context, created_network):"},{"line_number":474,"context_line":"        if self.network_scheduler:"},{"line_number":475,"context_line":"            return self.network_scheduler.schedule("}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_e8e70e0f","line":472,"updated":"2018-07-11 10:42:27.000000000","message":"how will this help if the controllers are on different hosts?\nwhy are you not doing it more refined - say per network id?\nthis means that you have just synchronized all of the scheduling","commit_id":"b19270aba69303fb0bff663a57099183b2357633"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"750714525f64600ffacd5eba8dfbee491db40752","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        else:"},{"line_number":470,"context_line":"            return {\u0027agents\u0027: []}"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    @runtime.synchronized(\u0027schedule-network\u0027, external\u003dTrue)"},{"line_number":473,"context_line":"    def schedule_network(self, context, created_network):"},{"line_number":474,"context_line":"        if self.network_scheduler:"},{"line_number":475,"context_line":"            return self.network_scheduler.schedule("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_9615cf3f","line":472,"in_reply_to":"5f7c97a3_53a6e056","updated":"2018-09-04 21:50:04.000000000","message":"If we really want to solve this problem in the scenario pointed out by garyk, we will need to go much finer than this with a \"compare and swap\" approach: Here.s a nice writeup of the theory behind it: https://www.joinfu.com/2015/01/understanding-reservations-concurrency-locking-in-nova/\n\nThis has been implemented in Neutron in the \u0027revision_number\u0027 attribute than many resources inherit from https://github.com/openstack/neutron/blob/master/neutron/db/standard_attr.py#L67. The other piece of this is the revisions plugin: https://github.com/openstack/neutron/blob/master/neutron/services/revisions/revision_plugin.py. To understand how this is implemented in SQLAlchemy, please look at http://docs.sqlalchemy.org/en/latest/orm/versioning.html.\n\nHaving said all this, I wonder what is the cost benefit of fixing this so we support the case indicated by garyk. In the bug you filed, you don\u0027t mention how badly your production environment / deployment is being affected. How bad is it? Also, I don\u0027t here other deployers complaining about this","commit_id":"b19270aba69303fb0bff663a57099183b2357633"},{"author":{"_account_id":28373,"name":"Kailun Qin","email":"kailun.qin@hotmail.com","username":"kailun.qin"},"change_message_id":"bf8c3a63f304b063fdae00c24de7d4e4cf9354ea","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        else:"},{"line_number":470,"context_line":"            return {\u0027agents\u0027: []}"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    @runtime.synchronized(\u0027schedule-network\u0027, external\u003dTrue)"},{"line_number":473,"context_line":"    def schedule_network(self, context, created_network):"},{"line_number":474,"context_line":"        if self.network_scheduler:"},{"line_number":475,"context_line":"            return self.network_scheduler.schedule("}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_53a6e056","line":472,"in_reply_to":"5f7c97a3_e8e70e0f","updated":"2018-07-12 07:53:00.000000000","message":"Thanks for the comments!\nMy two cents:\n1) When you say controllers on different hosts, do you mean a ha deployment for neutron services? If so, it seems that this doesn\u0027t help too much. Any idea?\n2) Getting a lock based on a finer granularity is a good idea for scheduling. I also thought a little about it and would like the review to decide how we act finally. However, for agent auto scheduling, we may also want the entire process to be atomic including the decision-making section which looks for non-hosted/non-scheduled resources. So I guess per network/router ID does not work that well.","commit_id":"b19270aba69303fb0bff663a57099183b2357633"}]}