)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1bed849a511cde0a0331f4df3b46eb952dd73576","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Support concurrently add hosts to aggregates"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"when add different host to one aggregate at the same time, the front host is rewrite by the back call."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Related-Bug: #1947813"},{"line_number":12,"context_line":"Change-Id: Iba48a969bb7f30cebbb08c34ed357b7d439b3090"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"cbcdf5ea_b4ef4d2a","line":10,"updated":"2021-11-10 16:31:48.000000000","message":"Could you please describe the scenario in a bit more detail when a race happens causing the issue? Also it would be nice to have a (functional) test that actually reproduces the race condition.","commit_id":"6c206b1f9bfa202aef885dd7c7a4226d0c5121aa"},{"author":{"_account_id":31412,"name":"Wenping Song","email":"songwenping@inspur.com","username":"songwenping"},"change_message_id":"55452dc68725129590ba9ac1aa9782386e0b3574","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Support concurrently add hosts to aggregates"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"when add different host to one aggregate at the same time, the front host is rewrite by the back call."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Related-Bug: #1947813"},{"line_number":12,"context_line":"Change-Id: Iba48a969bb7f30cebbb08c34ed357b7d439b3090"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bd775d78_47803346","line":10,"in_reply_to":"cbcdf5ea_b4ef4d2a","updated":"2021-12-13 12:22:23.000000000","message":"we support add more than one host to aggerate on the dashboard and send many aggegate/{host}/add_host apis to nova_api, nova_api get the same aggregate without host at[1], and cast nova_scheduler to update_aggregates for self.host_aggregates_map, the later host will replace the former host at [2].\n[1] https://review.opendev.org/plugins/gitiles/openstack/nova/+/refs/heads/master/nova/compute/api.py\n[2] https://review.opendev.org/plugins/gitiles/openstack/nova/+/refs/heads/master/nova/scheduler/host_manager.py","commit_id":"6c206b1f9bfa202aef885dd7c7a4226d0c5121aa"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":31412,"name":"Wenping Song","email":"songwenping@inspur.com","username":"songwenping"},"change_message_id":"48823780dbaf114933933b9bb7722c184b8bfa15","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9821116a_a6e0f7f6","updated":"2021-11-05 00:46:04.000000000","message":"recheck","commit_id":"6c206b1f9bfa202aef885dd7c7a4226d0c5121aa"}],"nova/scheduler/host_manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"20496e98d765760c873472302641b541fce42a5c","unresolved":true,"context_lines":[{"line_number":387,"context_line":"        for host in self.host_aggregates_map:"},{"line_number":388,"context_line":"            if (aggregate.id in self.host_aggregates_map[host] and"},{"line_number":389,"context_line":"                    host not in aggregate.hosts):"},{"line_number":390,"context_line":"                self.host_aggregates_map[host].remove(aggregate.id)"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    def delete_aggregate(self, aggregate):"},{"line_number":393,"context_line":"        \"\"\"Deletes internal HostManager information about a specific aggregate."}],"source_content_type":"text/x-python","patch_set":2,"id":"13b09b63_a43b5b8f","side":"PARENT","line":390,"updated":"2022-01-11 15:15:26.000000000","message":"\u003e we support add more than one host to aggerate on the dashboard and send many\n\u003e aggegate/{host}/add_host apis to nova_api, nova_api get the same aggregate without host\n\u003e at[1], and cast nova_scheduler to update_aggregates for self.host_aggregates_map, the\n\u003e later host will replace the former host at [2].\n\u003e [1] https://review.opendev.org/plugins/gitiles/openstack/nova/+/refs/heads/master/nova/compute/api.py\n\u003e [2] https://review.opendev.org/plugins/gitiles/openstack/nova/+/refs/heads/master/nova/scheduler/host_manager.py\n\n\nSo you described the race as following:\n* Client wants to add two hosts (host1, host2) to an aggregate. Lets assume for the current example that the aggregate has no hosts at the start.\n* Clients sends two aggegate/{host}/add_host requests (req1, req2) to nova\n* nova-api reads the aggregate from the DB. If the two requests are handled quickly (or in parallel) then both req1 and req2 reads the same, empty aggregate from the DB and nova-api adds host1 to the aggregate in req1 and host2 to the aggregate in req2. Then nova sends the both request to the scheduler req1(agg.hosts\u003d[host1]) req2(agg.hosts\u003d[host2])\n\nWith the lock you proposed to introduce the scheduler will handle req1 and req2 in sequence. \n\nLet\u0027s assume a sequence of req1, req2. Then\n* after req1: self.host_aggregates_map \u003d {host1: [agg]}\n* then after req2 will add host2: [agg] to the map but remove agg from host1. So self.host_aggregates_map \u003d {host2: [agg], host1: []}\n\nSo the lock you introduce here does not solve the race condition you described. \n\nI still suggest to provide a test case that reproduces the race first, then propose a fix.","commit_id":"e14eef0719eceef35e7e96b3e3d242ec79a80969"}]}