)]}'
{"nova/api/openstack/compute/server_groups.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6953952dae944176f3e409ec4e73b20b731d947","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        \"\"\"Creates a new server group.\"\"\""},{"line_number":119,"context_line":"        context \u003d _authorize_context(req, \u0027create\u0027)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        count \u003d objects.Quotas.count(context, \u0027server_groups\u0027,"},{"line_number":122,"context_line":"                                     context.project_id, context.user_id)"},{"line_number":123,"context_line":"        try:"},{"line_number":124,"context_line":"            objects.Quotas.limit_check(context, server_groups\u003dcount + 1)"},{"line_number":125,"context_line":"        except nova.exception.OverQuota:"},{"line_number":126,"context_line":"            msg \u003d _(\"Quota exceeded, too many server groups.\")"},{"line_number":127,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_818c8136","line":124,"range":{"start_line":121,"start_character":0,"end_line":124,"end_character":72},"updated":"2017-01-20 17:56:28.000000000","message":"Would this make sense as part of objects.InstanceGroup somehow? Its tempting to add it inside create(), so the read and write could live inside the same DB transaction?\n\nHaving said that, there is something nice about being explicit here.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2bbc86b2633fb2ad310955bdc86a5fbe3a0c8418","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        \"\"\"Creates a new server group.\"\"\""},{"line_number":119,"context_line":"        context \u003d _authorize_context(req, \u0027create\u0027)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        count \u003d objects.Quotas.count(context, \u0027server_groups\u0027,"},{"line_number":122,"context_line":"                                     context.project_id, context.user_id)"},{"line_number":123,"context_line":"        try:"},{"line_number":124,"context_line":"            objects.Quotas.limit_check(context, server_groups\u003dcount + 1)"},{"line_number":125,"context_line":"        except nova.exception.OverQuota:"},{"line_number":126,"context_line":"            msg \u003d _(\"Quota exceeded, too many server groups.\")"},{"line_number":127,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_b7429e3c","line":124,"range":{"start_line":121,"start_character":0,"end_line":124,"end_character":72},"in_reply_to":"5a3905b3_818c8136","updated":"2017-01-20 18:21:06.000000000","message":"I have wondered the same thing. I was trying to use the existing infra as much as possible to keep things more obvious, but the fact that these two checks are separate from the create transaction means it can race (for example, 3 parallel create requests, all get the same count, all pass limit_check, all get created even if over quota). To address that, dansmith and I were thinking of following each create with a \u0027destroy_and_raise_if_over_quota\u0027 (see objects/instance.py for an example).\n\nBut I think it would be simpler, easier to understand, and take care of the races to do everything in the create. Let me think on that.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0811b82cd4c29a79dac44a061355e4e668ef5c61","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        \"\"\"Creates a new server group.\"\"\""},{"line_number":119,"context_line":"        context \u003d _authorize_context(req, \u0027create\u0027)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        counts \u003d objects.Quotas.count(context, \u0027server_groups\u0027,"},{"line_number":122,"context_line":"                                      context.project_id, context.user_id)"},{"line_number":123,"context_line":"        try:"},{"line_number":124,"context_line":"            objects.Quotas.limit_check("},{"line_number":125,"context_line":"                context,"},{"line_number":126,"context_line":"                server_groups\u003dcounts[\u0027user\u0027] + 1,"},{"line_number":127,"context_line":"                proj_values\u003ddict(server_groups\u003dcounts[\u0027project\u0027] + 1))"},{"line_number":128,"context_line":"        except nova.exception.OverQuota:"},{"line_number":129,"context_line":"            msg \u003d _(\"Quota exceeded, too many server groups.\")"},{"line_number":130,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a461143_0278d0c3","line":127,"range":{"start_line":121,"start_character":0,"end_line":127,"end_character":70},"updated":"2017-01-26 17:15:33.000000000","message":"Honestly, I think we are missing a trick here.\n\nIdeally we would have a generation id on the Quota table, so we can make the limit_check atomic.\n\nIf we go for something like:\n\n    deltas \u003d [dict(server_groups\u003d1)]\n    objects.Quotas.check_deltas(context,\n        resource\u003d\"server_groups\",\n        deltas\u003ddeltas)\n\nNow we can totally implement that call as you have above, but it leaves us open to making it atomic later, and means the quota system is in change of extracting the details it needs from the context (I am thinking about nested quotas, etc).","commit_id":"e6612bcfbf526563a8cd4193ea29b6109e992db1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12ecc28fb2f3c414f8c18e1d22210da4197beb88","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        # Immediately try to delete to address quota races."},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            sg.destroy_and_raise_if_over_quota()"},{"line_number":146,"context_line":"        except nova.exception.OverQuota:"},{"line_number":147,"context_line":"            msg \u003d _(\"Quota exceeded, too many server groups.\")"},{"line_number":148,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_f99e03b7","line":145,"updated":"2017-01-26 17:35:45.000000000","message":"Could we not just do a limit check with deltas\u003d[] and if OverQuota is raised do sg.delete()?\n\nI guess that means when there is a burst of activity, a few too many folks might back off, but that seems OK, and simpler.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3133c08d0c4570e9dd47cb7ba3c46551f26784ff","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        # Immediately try to delete to address quota races."},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            sg.destroy_and_raise_if_over_quota()"},{"line_number":146,"context_line":"        except nova.exception.OverQuota:"},{"line_number":147,"context_line":"            msg \u003d _(\"Quota exceeded, too many server groups.\")"},{"line_number":148,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_ce47bc49","line":145,"in_reply_to":"3a461143_399deb59","updated":"2017-01-26 18:14:16.000000000","message":"Agreed with the use case and the problem.\n\nWhat I was thinking was here we just do:\n\n   try:\n       objects.Quotas.limit_check( no counts)\n   except nova.exception.OverQuota:\n       sg.destroy()\n        msg \u003d _(\"Quota exceeded, too many server groups.\")\n        raise exc.HTTPForbidden(explanation\u003dmsg)\n\nNow we loose the transaction security on the destroy, but it should be simpler to maintain that the nested query thing. Its probably less efficient SQL wise, but it leaves us with only one set of queries to optimize.\n\nI think your method means we are less likely to end up only creating 9, and failing 11. But I am not too worried about that case.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1f26e32885cd10e75da48bf02bd27e51149d3193","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        # Immediately try to delete to address quota races."},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            sg.destroy_and_raise_if_over_quota()"},{"line_number":146,"context_line":"        except nova.exception.OverQuota:"},{"line_number":147,"context_line":"            msg \u003d _(\"Quota exceeded, too many server groups.\")"},{"line_number":148,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_399deb59","line":145,"in_reply_to":"3a461143_f99e03b7","updated":"2017-01-26 17:44:08.000000000","message":"The scenario for this is: let\u0027s say the project-wide quota for server groups is 10 and there are no server groups in the project yet. If we get 20 server group create requests in parallel, all will count 0 at the same time, all will pass the limit check, and all 20 will be created, going 10 over quota.\n\nThis was a way to tie a count in with a delete, to keep the count/delete in a single db transaction. This way, not all 20 requests would succeed, only 10 of them.\n\nAnother way would be to count and create in a single db transaction. I\u0027m not sure which is more efficient as far as the sql queries involved.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"}],"nova/api/openstack/compute/tenant_networks.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ee24eb42169de1e9b058b6bf74758399113e21cc","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        try:"},{"line_number":157,"context_line":"            networks \u003d self.network_api.create(context,"},{"line_number":158,"context_line":"                                               label\u003dlabel, **kwargs)"},{"line_number":159,"context_line":"        except exception.PolicyNotAuthorized as e:"},{"line_number":160,"context_line":"            raise exc.HTTPForbidden(explanation\u003dsix.text_type(e))"},{"line_number":161,"context_line":"        except exception.CidrConflict as e:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_6ed24802","line":158,"updated":"2017-02-01 14:32:11.000000000","message":"Feels like we should be consistent and double check the quota after the create. Thats OK if we do it the easy way I guess.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"45dc19ee49d245008755c0278253e82c611be206","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        try:"},{"line_number":157,"context_line":"            networks \u003d self.network_api.create(context,"},{"line_number":158,"context_line":"                                               label\u003dlabel, **kwargs)"},{"line_number":159,"context_line":"        except exception.PolicyNotAuthorized as e:"},{"line_number":160,"context_line":"            raise exc.HTTPForbidden(explanation\u003dsix.text_type(e))"},{"line_number":161,"context_line":"        except exception.CidrConflict as e:"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa31d9ce_44acc2ed","line":158,"in_reply_to":"3a461143_6ed24802","updated":"2017-02-09 23:48:50.000000000","message":"True. I only did it for resource quotas that are not en-route to removal due to n-net deprecation. I was also thinking it was unlikely we\u0027d see parallel racing requests to create networks, but that might be a wrong assumption.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"}],"nova/compute/api.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4e45609f702283ed92f77512c8a1dc6abb084393","unresolved":false,"context_lines":[{"line_number":4420,"context_line":"        self.db.security_group_ensure_default(context)"},{"line_number":4421,"context_line":""},{"line_number":4422,"context_line":"    def create_security_group(self, context, name, description):"},{"line_number":4423,"context_line":"        count \u003d objects.Quotas.count(\u0027security_groups\u0027, context,"},{"line_number":4424,"context_line":"                                     context.project_id, context.user_id)"},{"line_number":4425,"context_line":"        try:"},{"line_number":4426,"context_line":"            objects.Quotas.limit_check(context, security_groups\u003dcount + 1)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_8242ad3c","line":4423,"range":{"start_line":4423,"start_character":37,"end_line":4423,"end_character":64},"updated":"2017-01-04 17:35:08.000000000","message":"These two args need to be swapped, they\u0027re in the wrong order.","commit_id":"2d8e51d116e0fa8c44fc54a64e0c075030e69d9b"},{"author":{"_account_id":23882,"name":"Dan Peschman","email":"dpeschman@godaddy.com","username":"dtp"},"change_message_id":"3b387874afd21900933b2cbb0be4f2e737ac980a","unresolved":false,"context_lines":[{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        # Check the quota"},{"line_number":302,"context_line":"        counts \u003d objects.Quotas.count(context, \u0027instances\u0027, project_id,"},{"line_number":303,"context_line":"                                      user_id)"},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            objects.Quotas.limit_check("},{"line_number":306,"context_line":"                context, instances\u003dcounts[\u0027instances\u0027] + max_count,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_fb2fff9e","line":303,"updated":"2017-01-13 14:04:02.000000000","message":"Is there a lock somewhere to prevent concurrent create requests from modifying usage after both have passed quota check?","commit_id":"f1ce97dcc63e078b8e4435cb98602eef4e25c979"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"60e2476d7660de9173ecf934a3e84d303d498d6c","unresolved":false,"context_lines":[{"line_number":1230,"context_line":"                instance \u003d build_request.get_new_instance(context)"},{"line_number":1231,"context_line":"                instance.create()"},{"line_number":1232,"context_line":"                # Immediately try to delete to address quota races."},{"line_number":1233,"context_line":"                instance.destroy_and_raise_if_over_quota()"},{"line_number":1234,"context_line":"                instances.append(instance)"},{"line_number":1235,"context_line":"                self._create_block_device_mapping("},{"line_number":1236,"context_line":"                    build_request.block_device_mappings)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_39b88be5","line":1233,"updated":"2017-01-26 17:41:22.000000000","message":"If you request 4 instances, and you only have quota for 3, don\u0027t we normally build 3 instances? I can\u0027t remember now, I am wondering if this changes that.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7313476099e6fdba9bfcc517652eb1e32105e8cd","unresolved":false,"context_lines":[{"line_number":1230,"context_line":"                instance \u003d build_request.get_new_instance(context)"},{"line_number":1231,"context_line":"                instance.create()"},{"line_number":1232,"context_line":"                # Immediately try to delete to address quota races."},{"line_number":1233,"context_line":"                instance.destroy_and_raise_if_over_quota()"},{"line_number":1234,"context_line":"                instances.append(instance)"},{"line_number":1235,"context_line":"                self._create_block_device_mapping("},{"line_number":1236,"context_line":"                    build_request.block_device_mappings)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_b9a63b51","line":1233,"in_reply_to":"3a461143_39b88be5","updated":"2017-01-26 17:55:26.000000000","message":"You mean with multi create? It depends on what min_count is. If min_count \u003e 3, the entire request will fail. If min_count \u003c\u003d 3 and max_count \u003d 4, the quota check will pass and I think what currently happens is 4 instances are created, 3 of them ACTIVE and the one over quota in ERROR state. This is a side effect of the fact that the over quota ones will fail to schedule.\n\nSo with this change, we won\u0027t get that happening anymore, it would preemptively delete those extra instances so nothing will be in ERROR state.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"239c5e45bae4fa5fc42379996c294a16d6439fee","unresolved":false,"context_lines":[{"line_number":2062,"context_line":"            instance.task_state \u003d task_states.RESTORING"},{"line_number":2063,"context_line":"            instance.deleted_at \u003d None"},{"line_number":2064,"context_line":"            instance.save(expected_task_state\u003d[None])"},{"line_number":2065,"context_line":"            self.compute_rpcapi.restore_instance(context, instance)"},{"line_number":2066,"context_line":"        else:"},{"line_number":2067,"context_line":"            instance.vm_state \u003d vm_states.ACTIVE"},{"line_number":2068,"context_line":"            instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_393fabd7","line":2065,"updated":"2017-01-26 18:04:13.000000000","message":"Maybe double check the quota before we sent the RPC message?\n\nI might be being paranoid.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"87682dc4ee85fe4956d26914d853f4aeba6a41b6","unresolved":false,"context_lines":[{"line_number":2062,"context_line":"            instance.task_state \u003d task_states.RESTORING"},{"line_number":2063,"context_line":"            instance.deleted_at \u003d None"},{"line_number":2064,"context_line":"            instance.save(expected_task_state\u003d[None])"},{"line_number":2065,"context_line":"            self.compute_rpcapi.restore_instance(context, instance)"},{"line_number":2066,"context_line":"        else:"},{"line_number":2067,"context_line":"            instance.vm_state \u003d vm_states.ACTIVE"},{"line_number":2068,"context_line":"            instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa31d9ce_999467e9","line":2065,"in_reply_to":"3a461143_393fabd7","updated":"2017-02-09 23:39:56.000000000","message":"Why double check instead of just moving the check later? Not sure I understand why to check twice.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"239c5e45bae4fa5fc42379996c294a16d6439fee","unresolved":false,"context_lines":[{"line_number":2067,"context_line":"            instance.vm_state \u003d vm_states.ACTIVE"},{"line_number":2068,"context_line":"            instance.task_state \u003d None"},{"line_number":2069,"context_line":"            instance.deleted_at \u003d None"},{"line_number":2070,"context_line":"            instance.save(expected_task_state\u003d[None])"},{"line_number":2071,"context_line":""},{"line_number":2072,"context_line":"    @check_instance_lock"},{"line_number":2073,"context_line":"    @check_instance_state(must_have_launched\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_99597f5d","line":2070,"updated":"2017-01-26 18:04:13.000000000","message":"Feels like we should double check the quota here.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"239c5e45bae4fa5fc42379996c294a16d6439fee","unresolved":false,"context_lines":[{"line_number":3065,"context_line":"        instance.task_state \u003d task_states.RESIZE_PREP"},{"line_number":3066,"context_line":"        instance.progress \u003d 0"},{"line_number":3067,"context_line":"        instance.update(extra_instance_updates)"},{"line_number":3068,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":3069,"context_line":""},{"line_number":3070,"context_line":"        filter_properties \u003d {\u0027ignore_hosts\u0027: []}"},{"line_number":3071,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_eb8f0a21","line":3068,"updated":"2017-01-26 18:04:13.000000000","message":"Oh, I wish we could update the flavor here, and then recheck the quota... but thats so much later on.\n\nI wonder when we count instances, should we look for any instances that are in some non-complete resize state, and if so check out instance.get_flavor(\u0027new\u0027) and add the delta to the count?\n\nI am stuck for efficient ideas really.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"87682dc4ee85fe4956d26914d853f4aeba6a41b6","unresolved":false,"context_lines":[{"line_number":3065,"context_line":"        instance.task_state \u003d task_states.RESIZE_PREP"},{"line_number":3066,"context_line":"        instance.progress \u003d 0"},{"line_number":3067,"context_line":"        instance.update(extra_instance_updates)"},{"line_number":3068,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":3069,"context_line":""},{"line_number":3070,"context_line":"        filter_properties \u003d {\u0027ignore_hosts\u0027: []}"},{"line_number":3071,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa31d9ce_395b7b56","line":3068,"in_reply_to":"3a461143_eb8f0a21","updated":"2017-02-09 23:39:56.000000000","message":"I see your point (as we discussed on IRC a couple of weeks ago). Let me look into what it would take to count resized-but-not-confirmed instances. It might just be a matter of tallying what is in the \u0027new_flavor\u0027 attribute on the Instance object.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"092451eb7bff9f99c39ea8425176219d11d55643","unresolved":false,"context_lines":[{"line_number":2843,"context_line":"        elevated \u003d context.elevated()"},{"line_number":2844,"context_line":"        migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":2845,"context_line":"            elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":2846,"context_line":""},{"line_number":2847,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":2848,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":2849,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_174ac9c5","line":2846,"updated":"2017-03-07 22:03:55.000000000","message":"QUESTION: The old code assumed the resize was an upsize. But for boot from volume, I think downsize is allowed. Therefore, a revert could result in an increase of resource consumption, so should we check quota here too?","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6193982379a2234e778397311f756a3e80ae08b2","unresolved":false,"context_lines":[{"line_number":2843,"context_line":"        elevated \u003d context.elevated()"},{"line_number":2844,"context_line":"        migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":2845,"context_line":"            elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":2846,"context_line":""},{"line_number":2847,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":2848,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":2849,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_f9b428f3","line":2846,"in_reply_to":"9a30ddce_174ac9c5","updated":"2017-03-08 15:55:24.000000000","message":"What would we do? Not allow them to revert? That would effectively lock down their instance until they made space for it to revert.\n\nI guess your concern is if they resize all their instances to small, then boot a bunch of new stuff, and then revert all their original things to end up massively oversubscribed?\n\nWhen we do the boot check of the new instances in the above scenario, are we comparing against the current values or the previous? We kinda need to check against the max of both I suppose. I wonder if that\u0027s doable?","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8871e861d056cc13d408ed87de5d337b315735a8","unresolved":false,"context_lines":[{"line_number":2843,"context_line":"        elevated \u003d context.elevated()"},{"line_number":2844,"context_line":"        migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":2845,"context_line":"            elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":2846,"context_line":""},{"line_number":2847,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":2848,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":2849,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_df2adf2b","line":2846,"in_reply_to":"9a30ddce_74075f1c","updated":"2017-03-08 18:31:29.000000000","message":"Yeah, I was thinking of a scenario where a user has resized down to a smaller flavor but didn\u0027t get confirm/revert. At this point, their instance is consuming fewer resources. (Recall that consumption gets updated BEFORE confirm/revert.) Then, they boot another instance and that succeeds, and they are now consuming all of their allowed quota.\n\nIf they revert the resize down instance, they would go over quota.\n\nI\u0027m not too concerned about it, it\u0027s just something I noticed and wanted to get some more thoughts from others, to help decide whether we should do something about it.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a1137e8e0d3f050f3a1431a4ece207ca5c2aa981","unresolved":false,"context_lines":[{"line_number":2843,"context_line":"        elevated \u003d context.elevated()"},{"line_number":2844,"context_line":"        migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":2845,"context_line":"            elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":2846,"context_line":""},{"line_number":2847,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":2848,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":2849,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_74075f1c","line":2846,"in_reply_to":"9a30ddce_f9b428f3","updated":"2017-03-08 16:36:47.000000000","message":"Maybe we should just move to what placement allocations should be held, i.e. we hold allocations for the new size and the old size during the resize.\n\nThe problem is if you resize down to get around a quota issue, that would fail. But maybe thats OK. It uses more quota during the resize, but you are using more resources, so maybe thats fine.\n\nSo on confirm or resize we end up just dropping the excess resource based on which instance gets deleted.\n\nIts different, but I think more accurate.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"092451eb7bff9f99c39ea8425176219d11d55643","unresolved":false,"context_lines":[{"line_number":2987,"context_line":"                    raise exception.TooManyInstances(overs\u003dovers,"},{"line_number":2988,"context_line":"                                                     req\u003dreqs,"},{"line_number":2989,"context_line":"                                                     used\u003duseds,"},{"line_number":2990,"context_line":"                                                     allowed\u003dtotal_alloweds)"},{"line_number":2991,"context_line":""},{"line_number":2992,"context_line":"        instance.task_state \u003d task_states.RESIZE_PREP"},{"line_number":2993,"context_line":"        instance.progress \u003d 0"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_57c7e111","line":2990,"updated":"2017-03-07 22:03:55.000000000","message":"QUESTION: For resize, the instance.vcpus, instance.memory_mb, and instance.flavor are not updated to the new flavor stats until finish_resize in nova-compute, which is an rpc cast. So how should we try to address races here? Recheck quota in confirm_resize and block it so the only option is to revert_resize?","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"da84bff33199114da1ee43a25cd466125272e2ec","unresolved":false,"context_lines":[{"line_number":2987,"context_line":"                    raise exception.TooManyInstances(overs\u003dovers,"},{"line_number":2988,"context_line":"                                                     req\u003dreqs,"},{"line_number":2989,"context_line":"                                                     used\u003duseds,"},{"line_number":2990,"context_line":"                                                     allowed\u003dtotal_alloweds)"},{"line_number":2991,"context_line":""},{"line_number":2992,"context_line":"        instance.task_state \u003d task_states.RESIZE_PREP"},{"line_number":2993,"context_line":"        instance.progress \u003d 0"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_ff4083a7","line":2990,"in_reply_to":"9a30ddce_4fc6464a","updated":"2017-03-08 18:38:44.000000000","message":"I\u0027m okay with that. Just wanted a sanity check from others. I agree it\u0027s a bad user experience to not be able to confirm/revert. So we can leave this and revert as-is.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6193982379a2234e778397311f756a3e80ae08b2","unresolved":false,"context_lines":[{"line_number":2987,"context_line":"                    raise exception.TooManyInstances(overs\u003dovers,"},{"line_number":2988,"context_line":"                                                     req\u003dreqs,"},{"line_number":2989,"context_line":"                                                     used\u003duseds,"},{"line_number":2990,"context_line":"                                                     allowed\u003dtotal_alloweds)"},{"line_number":2991,"context_line":""},{"line_number":2992,"context_line":"        instance.task_state \u003d task_states.RESIZE_PREP"},{"line_number":2993,"context_line":"        instance.progress \u003d 0"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_b9dd5016","line":2990,"in_reply_to":"9a30ddce_57c7e111","updated":"2017-03-08 15:55:24.000000000","message":"We could. I don\u0027t think blocking revert in the downsize case is doable for the reasons above, but I guess confirm maybe makes more sense. In the non-attack situation, it sucks for the user if they\u0027re allowed to resize, take the downtime, then later be told they can\u0027t confirm because of quota. That means they either have a locked instance stuck in that state, or have to take downtime again to revert back.\n\nI think this goes to how leaky (or not) we\u0027re okay with this being. Someone else should chime in.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a1137e8e0d3f050f3a1431a4ece207ca5c2aa981","unresolved":false,"context_lines":[{"line_number":2987,"context_line":"                    raise exception.TooManyInstances(overs\u003dovers,"},{"line_number":2988,"context_line":"                                                     req\u003dreqs,"},{"line_number":2989,"context_line":"                                                     used\u003duseds,"},{"line_number":2990,"context_line":"                                                     allowed\u003dtotal_alloweds)"},{"line_number":2991,"context_line":""},{"line_number":2992,"context_line":"        instance.task_state \u003d task_states.RESIZE_PREP"},{"line_number":2993,"context_line":"        instance.progress \u003d 0"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_4fc6464a","line":2990,"in_reply_to":"9a30ddce_b9dd5016","updated":"2017-03-08 16:36:47.000000000","message":"So an alternative to what I said about is to allow races here.\n\nJust do a check they are not over when you start the resize, and do no checks on confirm and revert.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3eb164aa7256b374e5e0844f352042578eb89801","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"MAX_USERDATA_SIZE \u003d 65535"},{"line_number":101,"context_line":"RO_SECURITY_GROUPS \u003d [\u0027default\u0027]"},{"line_number":102,"context_line":"VIDEO_RAM \u003d \u0027hw_video:ram_max_mb\u0027"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"AGGREGATE_ACTION_UPDATE \u003d \u0027Update\u0027"},{"line_number":105,"context_line":"AGGREGATE_ACTION_UPDATE_META \u003d \u0027UpdateMeta\u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a1ff146_0821b29f","side":"PARENT","line":102,"updated":"2017-03-16 09:20:46.000000000","message":"This seems like a bad rebase?","commit_id":"a90d736a556ea88a84c16bef83e2d27bf5e8d4ef"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"7992b2ca75c48a8b3e71a89cc97c07b5d097745a","unresolved":false,"context_lines":[{"line_number":3019,"context_line":"        cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":3020,"context_line":"        greenthreads \u003d []"},{"line_number":3021,"context_line":"        for cell_mapping in cell_mappings:"},{"line_number":3022,"context_line":"            with nova_context.target_cell(context, cell_mapping):"},{"line_number":3023,"context_line":"                # Need to make a copy of the context with db_connection set"},{"line_number":3024,"context_line":"                # before target_cell unsets it upon exit, so when the"},{"line_number":3025,"context_line":"                # greenthread does the db read, it reads from the intended"}],"source_content_type":"text/x-python","patch_set":46,"id":"5f201791_add26833","line":3022,"updated":"2017-06-20 19:32:21.000000000","message":"doesn\u0027t this have the same context problem that dan just ran into because there is no as ... clause?\n\nAlso, if the scatter/gather is going to be a common cells pattern is there a utility function to be made here?","commit_id":"dcc10268653a51144101279032cf05c2375bb7ed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"86e748bfe74480c62ee918f24b72dfdac76751ca","unresolved":false,"context_lines":[{"line_number":3019,"context_line":"        cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":3020,"context_line":"        greenthreads \u003d []"},{"line_number":3021,"context_line":"        for cell_mapping in cell_mappings:"},{"line_number":3022,"context_line":"            with nova_context.target_cell(context, cell_mapping):"},{"line_number":3023,"context_line":"                # Need to make a copy of the context with db_connection set"},{"line_number":3024,"context_line":"                # before target_cell unsets it upon exit, so when the"},{"line_number":3025,"context_line":"                # greenthread does the db read, it reads from the intended"}],"source_content_type":"text/x-python","patch_set":46,"id":"5f201791_2d7d381c","line":3022,"in_reply_to":"5f201791_8dc0cc52","updated":"2017-06-20 19:36:54.000000000","message":"Yeah, this change is from the before time when target_cell used to mutate the context passed in.","commit_id":"dcc10268653a51144101279032cf05c2375bb7ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7989806a96acf8622090060e7433fe62347680e3","unresolved":false,"context_lines":[{"line_number":3019,"context_line":"        cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":3020,"context_line":"        greenthreads \u003d []"},{"line_number":3021,"context_line":"        for cell_mapping in cell_mappings:"},{"line_number":3022,"context_line":"            with nova_context.target_cell(context, cell_mapping):"},{"line_number":3023,"context_line":"                # Need to make a copy of the context with db_connection set"},{"line_number":3024,"context_line":"                # before target_cell unsets it upon exit, so when the"},{"line_number":3025,"context_line":"                # greenthread does the db read, it reads from the intended"}],"source_content_type":"text/x-python","patch_set":46,"id":"5f201791_8dc0cc52","line":3022,"in_reply_to":"5f201791_add26833","updated":"2017-06-20 19:34:32.000000000","message":"Yes, same problem.\n\nAnd yes, this is a pattern we\u0027ll want to use elsewhere.","commit_id":"dcc10268653a51144101279032cf05c2375bb7ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7989806a96acf8622090060e7433fe62347680e3","unresolved":false,"context_lines":[{"line_number":3024,"context_line":"                # before target_cell unsets it upon exit, so when the"},{"line_number":3025,"context_line":"                # greenthread does the db read, it reads from the intended"},{"line_number":3026,"context_line":"                # cell database."},{"line_number":3027,"context_line":"                context_copy \u003d copy.copy(context)"},{"line_number":3028,"context_line":"                greenthreads.append(utils.spawn("},{"line_number":3029,"context_line":"                    objects.InstanceList.get_count_by_vm_state,"},{"line_number":3030,"context_line":"                    context_copy, project_id, user_id, vm_states.RESIZED))"}],"source_content_type":"text/x-python","patch_set":46,"id":"5f201791_4dbab4de","line":3027,"updated":"2017-06-20 19:34:32.000000000","message":"No need to copy this anymore because of how target_cell() works.","commit_id":"dcc10268653a51144101279032cf05c2375bb7ed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"86e748bfe74480c62ee918f24b72dfdac76751ca","unresolved":false,"context_lines":[{"line_number":3024,"context_line":"                # before target_cell unsets it upon exit, so when the"},{"line_number":3025,"context_line":"                # greenthread does the db read, it reads from the intended"},{"line_number":3026,"context_line":"                # cell database."},{"line_number":3027,"context_line":"                context_copy \u003d copy.copy(context)"},{"line_number":3028,"context_line":"                greenthreads.append(utils.spawn("},{"line_number":3029,"context_line":"                    objects.InstanceList.get_count_by_vm_state,"},{"line_number":3030,"context_line":"                    context_copy, project_id, user_id, vm_states.RESIZED))"}],"source_content_type":"text/x-python","patch_set":46,"id":"5f201791_ed76203f","line":3027,"in_reply_to":"5f201791_4dbab4de","updated":"2017-06-20 19:36:54.000000000","message":"Right.","commit_id":"dcc10268653a51144101279032cf05c2375bb7ed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            else:"},{"line_number":317,"context_line":"                raise exception.OnsetFileContentLimitExceeded()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    def _get_headroom(self, quotas, usages, deltas):"},{"line_number":320,"context_line":"        headroom \u003d {res: quotas[res] -"},{"line_number":321,"context_line":"                         (usages[res][\u0027in_use\u0027] + usages[res][\u0027reserved\u0027])"},{"line_number":322,"context_line":"                    for res in quotas.keys()}"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_1570eadb","side":"PARENT","line":319,"updated":"2017-07-11 20:55:09.000000000","message":"Tiny nit, but we could have done this refactor to move this stuff into compute.utils in a separate change to shorten up the size of this review a bit.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            else:"},{"line_number":317,"context_line":"                raise exception.OnsetFileContentLimitExceeded()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    def _get_headroom(self, quotas, usages, deltas):"},{"line_number":320,"context_line":"        headroom \u003d {res: quotas[res] -"},{"line_number":321,"context_line":"                         (usages[res][\u0027in_use\u0027] + usages[res][\u0027reserved\u0027])"},{"line_number":322,"context_line":"                    for res in quotas.keys()}"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_321ad27c","side":"PARENT","line":319,"in_reply_to":"3f1d235d_1570eadb","updated":"2017-07-12 02:27:38.000000000","message":"Ack.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fa3f2df8b58b23423b13087b2763099911cb9ac","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            else:"},{"line_number":317,"context_line":"                raise exception.OnsetFileContentLimitExceeded()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"    def _get_headroom(self, quotas, usages, deltas):"},{"line_number":320,"context_line":"        headroom \u003d {res: quotas[res] -"},{"line_number":321,"context_line":"                         (usages[res][\u0027in_use\u0027] + usages[res][\u0027reserved\u0027])"},{"line_number":322,"context_line":"                    for res in quotas.keys()}"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_9da46c0a","side":"PARENT","line":319,"in_reply_to":"3f1d235d_321ad27c","updated":"2017-07-12 21:52:13.000000000","message":"Note that I\u0027m not asking you to split this all up if it\u0027s a pain in the ass.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":1952,"context_line":"            instance.progress \u003d 0"},{"line_number":1953,"context_line":"            instance.save()"},{"line_number":1954,"context_line":""},{"line_number":1955,"context_line":"            # NOTE(comstud): If we delete the instance locally, we\u0027ll"},{"line_number":1956,"context_line":"            # commit the reservations here.  Otherwise, the manager side"},{"line_number":1957,"context_line":"            # will commit or rollback the reservations based on success."},{"line_number":1958,"context_line":"            quotas \u003d self._create_reservations(context,"},{"line_number":1959,"context_line":"                                               instance,"},{"line_number":1960,"context_line":"                                               original_task_state,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_a767cd51","side":"PARENT","line":1957,"range":{"start_line":1955,"start_character":12,"end_line":1957,"end_character":72},"updated":"2017-07-11 20:55:09.000000000","message":"So, what do we have to worry about with respect to older computes that aren\u0027t aware of all this counting quota jive turkey business going on in the API?\n\nI think based on this comment, we\u0027re good because now we no longer have reservations to commit or rollback, either here in the API or in the compute manager when the instance is deleted. Once someone upgrades to this code, they could have stale reservation records in the database, but it doesn\u0027t matter since we aren\u0027t going to read them anymore anyway.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":1952,"context_line":"            instance.progress \u003d 0"},{"line_number":1953,"context_line":"            instance.save()"},{"line_number":1954,"context_line":""},{"line_number":1955,"context_line":"            # NOTE(comstud): If we delete the instance locally, we\u0027ll"},{"line_number":1956,"context_line":"            # commit the reservations here.  Otherwise, the manager side"},{"line_number":1957,"context_line":"            # will commit or rollback the reservations based on success."},{"line_number":1958,"context_line":"            quotas \u003d self._create_reservations(context,"},{"line_number":1959,"context_line":"                                               instance,"},{"line_number":1960,"context_line":"                                               original_task_state,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_0f935dfc","side":"PARENT","line":1957,"range":{"start_line":1955,"start_character":12,"end_line":1957,"end_character":72},"in_reply_to":"1f1a1f67_262d2d95","updated":"2017-07-13 19:10:07.000000000","message":"++","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1952,"context_line":"            instance.progress \u003d 0"},{"line_number":1953,"context_line":"            instance.save()"},{"line_number":1954,"context_line":""},{"line_number":1955,"context_line":"            # NOTE(comstud): If we delete the instance locally, we\u0027ll"},{"line_number":1956,"context_line":"            # commit the reservations here.  Otherwise, the manager side"},{"line_number":1957,"context_line":"            # will commit or rollback the reservations based on success."},{"line_number":1958,"context_line":"            quotas \u003d self._create_reservations(context,"},{"line_number":1959,"context_line":"                                               instance,"},{"line_number":1960,"context_line":"                                               original_task_state,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_d5f5c8ee","side":"PARENT","line":1957,"range":{"start_line":1955,"start_character":12,"end_line":1957,"end_character":72},"in_reply_to":"3f1d235d_007e676a","updated":"2017-07-12 02:27:38.000000000","message":"Yeah, the usages table will be left in a pile of rubble which we shall not read.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d5c3cbf752563282a9884af8ab24a63d0f655b91","unresolved":false,"context_lines":[{"line_number":1952,"context_line":"            instance.progress \u003d 0"},{"line_number":1953,"context_line":"            instance.save()"},{"line_number":1954,"context_line":""},{"line_number":1955,"context_line":"            # NOTE(comstud): If we delete the instance locally, we\u0027ll"},{"line_number":1956,"context_line":"            # commit the reservations here.  Otherwise, the manager side"},{"line_number":1957,"context_line":"            # will commit or rollback the reservations based on success."},{"line_number":1958,"context_line":"            quotas \u003d self._create_reservations(context,"},{"line_number":1959,"context_line":"                                               instance,"},{"line_number":1960,"context_line":"                                               original_task_state,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_007e676a","side":"PARENT","line":1957,"range":{"start_line":1955,"start_character":12,"end_line":1957,"end_character":72},"in_reply_to":"3f1d235d_a767cd51","updated":"2017-07-11 21:20:44.000000000","message":"Yeah, I think we\u0027ll end up with all manner of nonsense in the usages table, but nothing should still be looking at it I think so it won\u0027t matter and we can drop those tables in R once we\u0027ve removed all model usage in queens.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":1952,"context_line":"            instance.progress \u003d 0"},{"line_number":1953,"context_line":"            instance.save()"},{"line_number":1954,"context_line":""},{"line_number":1955,"context_line":"            # NOTE(comstud): If we delete the instance locally, we\u0027ll"},{"line_number":1956,"context_line":"            # commit the reservations here.  Otherwise, the manager side"},{"line_number":1957,"context_line":"            # will commit or rollback the reservations based on success."},{"line_number":1958,"context_line":"            quotas \u003d self._create_reservations(context,"},{"line_number":1959,"context_line":"                                               instance,"},{"line_number":1960,"context_line":"                                               original_task_state,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_262d2d95","side":"PARENT","line":1957,"range":{"start_line":1955,"start_character":12,"end_line":1957,"end_character":72},"in_reply_to":"3f1d235d_d5f5c8ee","updated":"2017-07-13 15:22:42.000000000","message":"A pile of rubble is a suitable fate for this code, IMHO.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":2257,"context_line":"            instance.save()"},{"line_number":2258,"context_line":"        else:"},{"line_number":2259,"context_line":"            self.compute_rpcapi.terminate_instance(context, instance, bdms,"},{"line_number":2260,"context_line":"                                                   reservations\u003dreservations,"},{"line_number":2261,"context_line":"                                                   delete_type\u003d\u0027delete\u0027)"},{"line_number":2262,"context_line":""},{"line_number":2263,"context_line":"    def _do_force_delete(self, context, instance, bdms, reservations\u003dNone,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_474c799d","side":"PARENT","line":2260,"range":{"start_line":2260,"start_character":51,"end_line":2260,"end_character":76},"updated":"2017-07-11 20:55:09.000000000","message":"OK, so we actually got lucky with terminate_instance, because if you don\u0027t pass reservations, it defaults to None. The compute manager will create a Quotas object and then commit/rollback that, but the object code is a noop if there is no reservation to process:\n\nhttps://github.com/openstack/nova/blob/stable/ocata/nova/objects/quotas.py#L80\n\nAnd reservations is nullable\u003dTrue:\n\nhttps://github.com/openstack/nova/blob/stable/ocata/nova/objects/quotas.py#L57\n\nAnd the commit/rollback are noops:\n\nhttps://github.com/openstack/nova/blob/stable/ocata/nova/objects/quotas.py#L100\n\nhttps://github.com/openstack/nova/blob/stable/ocata/nova/objects/quotas.py#L111\n\nWhich is very fortunate for us otherwise we\u0027d have to consider older computes when not passing reservations down.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":2257,"context_line":"            instance.save()"},{"line_number":2258,"context_line":"        else:"},{"line_number":2259,"context_line":"            self.compute_rpcapi.terminate_instance(context, instance, bdms,"},{"line_number":2260,"context_line":"                                                   reservations\u003dreservations,"},{"line_number":2261,"context_line":"                                                   delete_type\u003d\u0027delete\u0027)"},{"line_number":2262,"context_line":""},{"line_number":2263,"context_line":"    def _do_force_delete(self, context, instance, bdms, reservations\u003dNone,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_cfbe8564","side":"PARENT","line":2260,"range":{"start_line":2260,"start_character":51,"end_line":2260,"end_character":76},"in_reply_to":"1f1a1f67_c6dc3976","updated":"2017-07-13 19:10:07.000000000","message":"Ack.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":2257,"context_line":"            instance.save()"},{"line_number":2258,"context_line":"        else:"},{"line_number":2259,"context_line":"            self.compute_rpcapi.terminate_instance(context, instance, bdms,"},{"line_number":2260,"context_line":"                                                   reservations\u003dreservations,"},{"line_number":2261,"context_line":"                                                   delete_type\u003d\u0027delete\u0027)"},{"line_number":2262,"context_line":""},{"line_number":2263,"context_line":"    def _do_force_delete(self, context, instance, bdms, reservations\u003dNone,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_f5cb24a2","side":"PARENT","line":2260,"range":{"start_line":2260,"start_character":51,"end_line":2260,"end_character":76},"in_reply_to":"3f1d235d_474c799d","updated":"2017-07-12 02:27:38.000000000","message":"Yeah, we lucked out on all the no-op cleverness in quotas.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":2257,"context_line":"            instance.save()"},{"line_number":2258,"context_line":"        else:"},{"line_number":2259,"context_line":"            self.compute_rpcapi.terminate_instance(context, instance, bdms,"},{"line_number":2260,"context_line":"                                                   reservations\u003dreservations,"},{"line_number":2261,"context_line":"                                                   delete_type\u003d\u0027delete\u0027)"},{"line_number":2262,"context_line":""},{"line_number":2263,"context_line":"    def _do_force_delete(self, context, instance, bdms, reservations\u003dNone,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_c6dc3976","side":"PARENT","line":2260,"range":{"start_line":2260,"start_character":51,"end_line":2260,"end_character":76},"in_reply_to":"3f1d235d_f5cb24a2","updated":"2017-07-13 15:22:42.000000000","message":"Yeah, we should follow up and remove the param from the rpcapi side, always passing None and making a note to remove those things when we do the next bump.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":2280,"context_line":"            instance.terminated_at \u003d timeutils.utcnow()"},{"line_number":2281,"context_line":"            instance.save()"},{"line_number":2282,"context_line":"        else:"},{"line_number":2283,"context_line":"            self.compute_rpcapi.soft_delete_instance(context, instance,"},{"line_number":2284,"context_line":"                                                     reservations\u003dreservations)"},{"line_number":2285,"context_line":""},{"line_number":2286,"context_line":"    # NOTE(maoy): we allow delete to be called no matter what vm_state says."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_8759715b","side":"PARENT","line":2283,"range":{"start_line":2283,"start_character":32,"end_line":2283,"end_character":52},"updated":"2017-07-11 20:55:09.000000000","message":"same situation as terminate_instance above","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":3226,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":3227,"context_line":"                elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":3228,"context_line":""},{"line_number":3229,"context_line":"        # reserve quota only for any decrease in resource usage"},{"line_number":3230,"context_line":"        deltas \u003d compute_utils.downsize_quota_delta(context, instance)"},{"line_number":3231,"context_line":"        quotas \u003d compute_utils.reserve_quota_delta(context, deltas, instance)"},{"line_number":3232,"context_line":""},{"line_number":3233,"context_line":"        migration.status \u003d \u0027confirming\u0027"},{"line_number":3234,"context_line":"        migration.save()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_dd2a982a","side":"PARENT","line":3231,"range":{"start_line":3229,"start_character":8,"end_line":3231,"end_character":77},"updated":"2017-07-11 20:55:09.000000000","message":"So we don\u0027t check usage because we did that when the resize was initiated.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":3226,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":3227,"context_line":"                elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":3228,"context_line":""},{"line_number":3229,"context_line":"        # reserve quota only for any decrease in resource usage"},{"line_number":3230,"context_line":"        deltas \u003d compute_utils.downsize_quota_delta(context, instance)"},{"line_number":3231,"context_line":"        quotas \u003d compute_utils.reserve_quota_delta(context, deltas, instance)"},{"line_number":3232,"context_line":""},{"line_number":3233,"context_line":"        migration.status \u003d \u0027confirming\u0027"},{"line_number":3234,"context_line":"        migration.save()"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_efa6e153","side":"PARENT","line":3231,"range":{"start_line":3229,"start_character":8,"end_line":3231,"end_character":77},"in_reply_to":"1f1a1f67_fac241e9","updated":"2017-07-13 19:10:07.000000000","message":"As discussed on IRC, we\u0027re going to go ahead and not change the behavior with the counting quotas series. The next patch (limit pending resizes) can go away too.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee9fa5442d520c7a6dba03845fb5faff4de3336d","unresolved":false,"context_lines":[{"line_number":3226,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":3227,"context_line":"                elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":3228,"context_line":""},{"line_number":3229,"context_line":"        # reserve quota only for any decrease in resource usage"},{"line_number":3230,"context_line":"        deltas \u003d compute_utils.downsize_quota_delta(context, instance)"},{"line_number":3231,"context_line":"        quotas \u003d compute_utils.reserve_quota_delta(context, deltas, instance)"},{"line_number":3232,"context_line":""},{"line_number":3233,"context_line":"        migration.status \u003d \u0027confirming\u0027"},{"line_number":3234,"context_line":"        migration.save()"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_fac241e9","side":"PARENT","line":3231,"range":{"start_line":3229,"start_character":8,"end_line":3231,"end_character":77},"in_reply_to":"3f1d235d_d5114806","updated":"2017-07-13 01:54:41.000000000","message":"\u003e Although, it looks like current behavior would be willing to block a confirm or revert. So I\u0027m not sure why we decided to change the behavior here. Maybe because rejecting a confirm or revert is crappy.\n\nYeah, I was wondering the same - if you can fail a confirm resize by going over quota, then why would we change that behavior here once we start counting? It seems like this should just be a check_deltas call on the adjusted deltas.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":3226,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":3227,"context_line":"                elevated, instance.uuid, \u0027finished\u0027)"},{"line_number":3228,"context_line":""},{"line_number":3229,"context_line":"        # reserve quota only for any decrease in resource usage"},{"line_number":3230,"context_line":"        deltas \u003d compute_utils.downsize_quota_delta(context, instance)"},{"line_number":3231,"context_line":"        quotas \u003d compute_utils.reserve_quota_delta(context, deltas, instance)"},{"line_number":3232,"context_line":""},{"line_number":3233,"context_line":"        migration.status \u003d \u0027confirming\u0027"},{"line_number":3234,"context_line":"        migration.save()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_d5114806","side":"PARENT","line":3231,"range":{"start_line":3229,"start_character":8,"end_line":3231,"end_character":77},"in_reply_to":"3f1d235d_dd2a982a","updated":"2017-07-12 02:27:38.000000000","message":"Right. This will be related to the next patch in the series, where we limit the number of unconfirmed/unreverted resizes and not check quota on confirm/revert. We don\u0027t want to ever block a confirm or revert. Although, it looks like current behavior would be willing to block a confirm or revert. So I\u0027m not sure why we decided to change the behavior here. Maybe because rejecting a confirm or revert is crappy.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"        # reservations immediately..."},{"line_number":3255,"context_line":"        quotas.commit()"},{"line_number":3256,"context_line":"        # NOTE(johannes/comstud): The API cell needs a local migration"},{"line_number":3257,"context_line":"        # record for later resize_confirm and resize_reverts to deal"},{"line_number":3258,"context_line":"        # with quotas.  We don\u0027t need source and/or destination"},{"line_number":3259,"context_line":"        # information, just the old and new flavors. Status is set to"},{"line_number":3260,"context_line":"        # \u0027finished\u0027 since nothing else will update the status along"},{"line_number":3261,"context_line":"        # the way."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_1dd6d028","side":"PARENT","line":3258,"range":{"start_line":3257,"start_character":60,"end_line":3258,"end_character":22},"updated":"2017-07-11 20:55:09.000000000","message":"If the migration record is only needed for dealing with quotas, and we are no longer dealing with quotas, do we need to crate this migration record for cells v1 still?","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"        # reservations immediately..."},{"line_number":3255,"context_line":"        quotas.commit()"},{"line_number":3256,"context_line":"        # NOTE(johannes/comstud): The API cell needs a local migration"},{"line_number":3257,"context_line":"        # record for later resize_confirm and resize_reverts to deal"},{"line_number":3258,"context_line":"        # with quotas.  We don\u0027t need source and/or destination"},{"line_number":3259,"context_line":"        # information, just the old and new flavors. Status is set to"},{"line_number":3260,"context_line":"        # \u0027finished\u0027 since nothing else will update the status along"},{"line_number":3261,"context_line":"        # the way."}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_0f5a3d4e","side":"PARENT","line":3258,"range":{"start_line":3257,"start_character":60,"end_line":3258,"end_character":22},"in_reply_to":"1f1a1f67_e65e15e5","updated":"2017-07-13 19:10:07.000000000","message":"Okay, I wasn\u0027t aware that it had come up before outside the counting quotas discussion. If there\u0027s a separate use case for that, I agree it should be its own quota. There could be a separate, later spec on that in the future.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d5c3cbf752563282a9884af8ab24a63d0f655b91","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"        # reservations immediately..."},{"line_number":3255,"context_line":"        quotas.commit()"},{"line_number":3256,"context_line":"        # NOTE(johannes/comstud): The API cell needs a local migration"},{"line_number":3257,"context_line":"        # record for later resize_confirm and resize_reverts to deal"},{"line_number":3258,"context_line":"        # with quotas.  We don\u0027t need source and/or destination"},{"line_number":3259,"context_line":"        # information, just the old and new flavors. Status is set to"},{"line_number":3260,"context_line":"        # \u0027finished\u0027 since nothing else will update the status along"},{"line_number":3261,"context_line":"        # the way."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_a0d7b34f","side":"PARENT","line":3258,"range":{"start_line":3257,"start_character":60,"end_line":3258,"end_character":22},"in_reply_to":"3f1d235d_1dd6d028","updated":"2017-07-11 21:20:44.000000000","message":"I would think it\u0027d still be wrong to not have a migration record at the top-level cell. I\u0027d have to go look for something that depends on it in the API (aside from actually listing migrations) but I think it\u0027s less change to just leave it. I think this comment is talking about mechanical needs for the migration record, not specifically quota-related things right?","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"        # reservations immediately..."},{"line_number":3255,"context_line":"        quotas.commit()"},{"line_number":3256,"context_line":"        # NOTE(johannes/comstud): The API cell needs a local migration"},{"line_number":3257,"context_line":"        # record for later resize_confirm and resize_reverts to deal"},{"line_number":3258,"context_line":"        # with quotas.  We don\u0027t need source and/or destination"},{"line_number":3259,"context_line":"        # information, just the old and new flavors. Status is set to"},{"line_number":3260,"context_line":"        # \u0027finished\u0027 since nothing else will update the status along"},{"line_number":3261,"context_line":"        # the way."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_b5948c45","side":"PARENT","line":3258,"range":{"start_line":3257,"start_character":60,"end_line":3258,"end_character":22},"in_reply_to":"3f1d235d_8c91909c","updated":"2017-07-12 02:27:38.000000000","message":"Yeah, it appears the migration record was only needed for the old and new flavor sizes to do a quota check during confirm/revert. But based on discussions had so far with John at least, we were thinking we shouldn\u0027t block anyone from confirming or reverting a resize. And instead we should limit the number (via config) of \"pending resizes\" that have neither been confirmed nor reverted, the limit the magnitude of quota violations.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1b9407c4ec98fbb51c35eaf00d891b1245fef153","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"        # reservations immediately..."},{"line_number":3255,"context_line":"        quotas.commit()"},{"line_number":3256,"context_line":"        # NOTE(johannes/comstud): The API cell needs a local migration"},{"line_number":3257,"context_line":"        # record for later resize_confirm and resize_reverts to deal"},{"line_number":3258,"context_line":"        # with quotas.  We don\u0027t need source and/or destination"},{"line_number":3259,"context_line":"        # information, just the old and new flavors. Status is set to"},{"line_number":3260,"context_line":"        # \u0027finished\u0027 since nothing else will update the status along"},{"line_number":3261,"context_line":"        # the way."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_8c91909c","side":"PARENT","line":3258,"range":{"start_line":3257,"start_character":60,"end_line":3258,"end_character":22},"in_reply_to":"3f1d235d_a0d7b34f","updated":"2017-07-12 00:22:58.000000000","message":"idk, but I\u0027m also totally fine with just continuing to create the migration record for now, was just random musing. It can always die when we kill cells v1.","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"        # reservations immediately..."},{"line_number":3255,"context_line":"        quotas.commit()"},{"line_number":3256,"context_line":"        # NOTE(johannes/comstud): The API cell needs a local migration"},{"line_number":3257,"context_line":"        # record for later resize_confirm and resize_reverts to deal"},{"line_number":3258,"context_line":"        # with quotas.  We don\u0027t need source and/or destination"},{"line_number":3259,"context_line":"        # information, just the old and new flavors. Status is set to"},{"line_number":3260,"context_line":"        # \u0027finished\u0027 since nothing else will update the status along"},{"line_number":3261,"context_line":"        # the way."}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_e65e15e5","side":"PARENT","line":3258,"range":{"start_line":3257,"start_character":60,"end_line":3258,"end_character":22},"in_reply_to":"3f1d235d_b5948c45","updated":"2017-07-13 15:22:42.000000000","message":"Yeah I think people have asked for a max_pending_resizes limit anyway. Kinda seems like that needs to be a quota and not a config thing, and it\u0027d be easy to count but...","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # Check quotas"},{"line_number":886,"context_line":"        num_instances \u003d compute_utils.check_num_instances_quota("},{"line_number":887,"context_line":"                context, instance_type, min_count, max_count,"},{"line_number":888,"context_line":"                project_id\u003dcontext.project_id, user_id\u003dcontext.user_id)"},{"line_number":889,"context_line":"        security_groups \u003d self.security_group_api.populate_security_groups("},{"line_number":890,"context_line":"                security_groups)"},{"line_number":891,"context_line":"        self.security_group_api.ensure_default(context)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_1a49398a","line":888,"range":{"start_line":888,"start_character":16,"end_line":888,"end_character":70},"updated":"2017-07-11 20:55:09.000000000","message":"These will default to the same values in limit_check_project_and_user in the quota engine so it doesn\u0027t seem like we should care here about overrides.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0cb595f39887de81c7e2536988cbe12ecb3a281b","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # Check quotas"},{"line_number":886,"context_line":"        num_instances \u003d compute_utils.check_num_instances_quota("},{"line_number":887,"context_line":"                context, instance_type, min_count, max_count,"},{"line_number":888,"context_line":"                project_id\u003dcontext.project_id, user_id\u003dcontext.user_id)"},{"line_number":889,"context_line":"        security_groups \u003d self.security_group_api.populate_security_groups("},{"line_number":890,"context_line":"                security_groups)"},{"line_number":891,"context_line":"        self.security_group_api.ensure_default(context)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_383d66bb","line":888,"range":{"start_line":888,"start_character":16,"end_line":888,"end_character":70},"in_reply_to":"1f1a1f67_9d7d8c70","updated":"2017-07-12 22:20:58.000000000","message":"Yep, thanks.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # Check quotas"},{"line_number":886,"context_line":"        num_instances \u003d compute_utils.check_num_instances_quota("},{"line_number":887,"context_line":"                context, instance_type, min_count, max_count,"},{"line_number":888,"context_line":"                project_id\u003dcontext.project_id, user_id\u003dcontext.user_id)"},{"line_number":889,"context_line":"        security_groups \u003d self.security_group_api.populate_security_groups("},{"line_number":890,"context_line":"                security_groups)"},{"line_number":891,"context_line":"        self.security_group_api.ensure_default(context)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_f2133a65","line":888,"range":{"start_line":888,"start_character":16,"end_line":888,"end_character":70},"in_reply_to":"3f1d235d_1a49398a","updated":"2017-07-12 02:27:38.000000000","message":"True that. Not sure why I overrode them.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fa3f2df8b58b23423b13087b2763099911cb9ac","unresolved":false,"context_lines":[{"line_number":885,"context_line":"        # Check quotas"},{"line_number":886,"context_line":"        num_instances \u003d compute_utils.check_num_instances_quota("},{"line_number":887,"context_line":"                context, instance_type, min_count, max_count,"},{"line_number":888,"context_line":"                project_id\u003dcontext.project_id, user_id\u003dcontext.user_id)"},{"line_number":889,"context_line":"        security_groups \u003d self.security_group_api.populate_security_groups("},{"line_number":890,"context_line":"                security_groups)"},{"line_number":891,"context_line":"        self.security_group_api.ensure_default(context)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_9d7d8c70","line":888,"range":{"start_line":888,"start_character":16,"end_line":888,"end_character":70},"in_reply_to":"3f1d235d_f2133a65","updated":"2017-07-12 21:52:13.000000000","message":"check_num_instances_quota in compute utils will need to default project_id and user_id if they aren\u0027t provided though, so you\u0027d have to account for that I think. The quota engine does that itself, but there is other stuff in check_num_instances_quota that uses the kwargs.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":1156,"context_line":"                    try:"},{"line_number":1157,"context_line":"                        compute_utils.check_num_instances_quota("},{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_5509c292","line":1160,"range":{"start_line":1159,"start_character":28,"end_line":1160,"end_character":51},"updated":"2017-07-11 20:55:09.000000000","message":"redundant","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1156,"context_line":"                    try:"},{"line_number":1157,"context_line":"                        compute_utils.check_num_instances_quota("},{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_751cb469","line":1160,"range":{"start_line":1159,"start_character":28,"end_line":1160,"end_character":51},"in_reply_to":"3f1d235d_5509c292","updated":"2017-07-12 02:27:38.000000000","message":"Ack.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fa3f2df8b58b23423b13087b2763099911cb9ac","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_bdc04852","line":1161,"range":{"start_line":1161,"start_character":37,"end_line":1161,"end_character":53},"updated":"2017-07-12 21:52:13.000000000","message":"This is going to result in a 500 from the API because the REST controller doesn\u0027t handle this type of exception. We should probably convert and raise OverQuota which is handled and will result in a 403.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3d6794617d153ddb2656c205234e105cce7f9ea9","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_3760d4ce","line":1161,"range":{"start_line":1161,"start_character":37,"end_line":1161,"end_character":53},"in_reply_to":"1f1a1f67_136ded97","updated":"2017-07-13 00:00:05.000000000","message":"Good point, I was thinking the REST controller handled OverQuota, but it\u0027s QuotaError as you said, and I didn\u0027t realize that\u0027s a super class for TooManyInstances.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4952f25a1ebcea5527ebb33115f3379d76e339f8","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_136ded97","line":1161,"range":{"start_line":1161,"start_character":37,"end_line":1161,"end_character":53},"in_reply_to":"1f1a1f67_bdc04852","updated":"2017-07-12 23:34:35.000000000","message":"I think it doesn\u0027t because TooManyInstances derives from QuotaError and QuotaError is handled in several APIs.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"},{"line_number":1165,"context_line":"                instances\u003dinstances, image\u003dboot_meta,"},{"line_number":1166,"context_line":"                filter_properties\u003dfilter_properties,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_958d1ad5","line":1163,"range":{"start_line":1161,"start_character":20,"end_line":1163,"end_character":46},"updated":"2017-07-11 20:55:09.000000000","message":"I was thinking at first we might need to pull the \u0027overs\u0027 out of the exception to see how many instances we need to delete, but since we check this each time after we create an instance, it should be a tight check and we should only go over by one, at least in this request. We could be hitting concurrent requests, but the concurrent request should fail if we\u0027ve already hit the limit. I was just wondering if there was a case where we\u0027ve gone over by more than one and needed to be deleting more, but if overs is \u003e 1 due to parallel requests, then it seems each request should be in charge of cleaning up after itself, after the instance.create() call. I might just be over-thinking this.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"},{"line_number":1165,"context_line":"                instances\u003dinstances, image\u003dboot_meta,"},{"line_number":1166,"context_line":"                filter_properties\u003dfilter_properties,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_8f9f6d02","line":1163,"range":{"start_line":1161,"start_character":20,"end_line":1163,"end_character":46},"in_reply_to":"1f1a1f67_0b3b5aeb","updated":"2017-07-13 19:10:07.000000000","message":"Ugh ... yeah, good point. What we really need here is instance hard-delete. Would still have the split-second problem with that too though.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"},{"line_number":1165,"context_line":"                instances\u003dinstances, image\u003dboot_meta,"},{"line_number":1166,"context_line":"                filter_properties\u003dfilter_properties,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_0b3b5aeb","line":1163,"range":{"start_line":1161,"start_character":20,"end_line":1163,"end_character":46},"in_reply_to":"1f1a1f67_b31e396e","updated":"2017-07-13 15:22:42.000000000","message":"In reality, instance.destroy() isn\u0027t really what we want either, because that will leave a shell in the database visible by things that can read deleted rows. Meaning, before this counting change we wouldn\u0027t have ended up with an instance in the database by the time we got a quota error, but now we end up with a deleted one. The same argument applies to the fact that we end up with an instance for a split second if someone was doing a list in a tight loop or something.\n\nI dunno how problematic that really is, but I could think of a couple of ways around it. Probably shouldn\u0027t worry about it here, and/or until someone raises it as an issue though.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5622a6923f922dd1281a449adb655860fd9a5e96","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"},{"line_number":1165,"context_line":"                instances\u003dinstances, image\u003dboot_meta,"},{"line_number":1166,"context_line":"                filter_properties\u003dfilter_properties,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_b31e396e","line":1163,"range":{"start_line":1161,"start_character":20,"end_line":1163,"end_character":46},"in_reply_to":"1f1a1f67_bd64c8ba","updated":"2017-07-12 23:48:44.000000000","message":"As discussed on IRC, I think you\u0027re right that we need to clean up all traces of the objects we created as part of this request (instances, build requests, request specs, instance mappings) because we haven\u0027t yet returned to the user. They\u0027ll get a 403 and we should not have any remnants of the request objects.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fa3f2df8b58b23423b13087b2763099911cb9ac","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"},{"line_number":1165,"context_line":"                instances\u003dinstances, image\u003dboot_meta,"},{"line_number":1166,"context_line":"                filter_properties\u003dfilter_properties,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_bd64c8ba","line":1163,"range":{"start_line":1161,"start_character":20,"end_line":1163,"end_character":46},"in_reply_to":"3f1d235d_953630e7","updated":"2017-07-12 21:52:13.000000000","message":"\u003e Since you mentioned it though, I\u0027m not 100% sure what will be done with the remaining instance records that may have been created prior to going over quota.\n\nBut don\u0027t we count and check quotas for the number of instances that we\u0027re going to create before we ever create any? So if we were going to go over quota, I\u0027d think we\u0027d have done it by now. Unless we had something like:\n\n1. limit is 10, usage is 8\n2. request A comes in to create 2 instances, check is ok as we have headroom for 2 more instances\n3. concurrent request B comes in to create 2 instances, check is OK as we have headroom for 2 more instances\n4. request A creates its first instance and does a recheck, which is OK because we\u0027re at 9 for usage\n5. request B creates its first instance and does a recheck,\nwhich is OK because we\u0027re at 10 for usage (limit is 10)\n6. request A creates its 2nd instance and does a recheck, which fails because we\u0027re now at usage\u003d11, so it deletes that 2nd instance from request A. It raises TooManyInstances but the first instance from request A still exists, and isn\u0027t put into error state, it\u0027s just stuck in build state.\n7. The same thing is going to happen in request B\u0027s second instance as in #6 above.\n\nSo it seems here, if we go over at all, we have to do something about all of the instances that we created, which I\u0027m guessing is ERROR state. Or do we delete them and raise TooManyInstances which results in a 403 (like OverQuota would)?\n\nThe way this was dealt with before was request A would create a reservation for 2 instances, which would put usage at 10 (8 in_use, 2 reserved), so when request B tried to create a reservation for 2 more instances, it would be over quota and fail.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1158,"context_line":"                            context, instance_type, 0, 0,"},{"line_number":1159,"context_line":"                            project_id\u003dcontext.project_id,"},{"line_number":1160,"context_line":"                            user_id\u003dcontext.user_id)"},{"line_number":1161,"context_line":"                    except exception.TooManyInstances:"},{"line_number":1162,"context_line":"                        with excutils.save_and_reraise_exception():"},{"line_number":1163,"context_line":"                            instance.destroy()"},{"line_number":1164,"context_line":"            self.compute_task_api.build_instances(context,"},{"line_number":1165,"context_line":"                instances\u003dinstances, image\u003dboot_meta,"},{"line_number":1166,"context_line":"                filter_properties\u003dfilter_properties,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_953630e7","line":1163,"range":{"start_line":1161,"start_character":20,"end_line":1163,"end_character":46},"in_reply_to":"3f1d235d_958d1ad5","updated":"2017-07-12 02:27:38.000000000","message":"Yeah, that\u0027s what I was thinking, to match up each create() with a destroy(). Since you mentioned it though, I\u0027m not 100% sure what will be done with the remaining instance records that may have been created prior to going over quota. They should be in ERROR state. I need to check that.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":1685,"context_line":"            # the conductor."},{"line_number":1686,"context_line":""},{"line_number":1687,"context_line":"            # NOTE(alaski): Though the conductor halts the build process it"},{"line_number":1688,"context_line":"            # does not currently delete the instance record. This is"},{"line_number":1689,"context_line":"            # because in the near future the instance record will not be"},{"line_number":1690,"context_line":"            # created if the buildrequest has been deleted here. For now we"},{"line_number":1691,"context_line":"            # ensure the instance has been set to deleted at this point."},{"line_number":1692,"context_line":"            # Yes this directly contradicts the comment earlier in this"},{"line_number":1693,"context_line":"            # method, but this is a temporary measure."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_87bf31fa","line":1690,"range":{"start_line":1688,"start_character":61,"end_line":1690,"end_character":64},"updated":"2017-07-11 20:55:09.000000000","message":"Unrelated to this change, but I\u0027m still a bit troubled by the fact we have this code in here because I\u0027m pretty sure \"near future\" was written before we started creating the instance in the conductor and did cleanup handling in there. We should probably remember to never say things about \"near future\" or other relative times in code again, since this near future is about a year old now.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1685,"context_line":"            # the conductor."},{"line_number":1686,"context_line":""},{"line_number":1687,"context_line":"            # NOTE(alaski): Though the conductor halts the build process it"},{"line_number":1688,"context_line":"            # does not currently delete the instance record. This is"},{"line_number":1689,"context_line":"            # because in the near future the instance record will not be"},{"line_number":1690,"context_line":"            # created if the buildrequest has been deleted here. For now we"},{"line_number":1691,"context_line":"            # ensure the instance has been set to deleted at this point."},{"line_number":1692,"context_line":"            # Yes this directly contradicts the comment earlier in this"},{"line_number":1693,"context_line":"            # method, but this is a temporary measure."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_d5a7080b","line":1690,"range":{"start_line":1688,"start_character":61,"end_line":1690,"end_character":64},"in_reply_to":"3f1d235d_87bf31fa","updated":"2017-07-12 02:27:38.000000000","message":"Agreed.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":1690,"context_line":"            # created if the buildrequest has been deleted here. For now we"},{"line_number":1691,"context_line":"            # ensure the instance has been set to deleted at this point."},{"line_number":1692,"context_line":"            # Yes this directly contradicts the comment earlier in this"},{"line_number":1693,"context_line":"            # method, but this is a temporary measure."},{"line_number":1694,"context_line":"            # Look up the instance because the current instance object was"},{"line_number":1695,"context_line":"            # stashed on the buildrequest and therefore not complete enough"},{"line_number":1696,"context_line":"            # to run .destroy()."}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_4737d9b1","line":1693,"range":{"start_line":1693,"start_character":22,"end_line":1693,"end_character":54},"updated":"2017-07-11 20:55:09.000000000","message":"same","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":1708,"context_line":"                                instance.destroy()"},{"line_number":1709,"context_line":"                    else:"},{"line_number":1710,"context_line":"                        instance.destroy()"},{"line_number":1711,"context_line":"                else:"},{"line_number":1712,"context_line":"                    # The instance is already deleted so rollback the quota"},{"line_number":1713,"context_line":"                    # usage decrement reservation in the not found block below."},{"line_number":1714,"context_line":"                    raise exception.InstanceNotFound(instance_id\u003dinstance_uuid)"},{"line_number":1715,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1716,"context_line":"                pass"},{"line_number":1717,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_67b3f54e","line":1714,"range":{"start_line":1711,"start_character":16,"end_line":1714,"end_character":79},"updated":"2017-07-11 20:55:09.000000000","message":"Looks like we can remove this, as this was just taking advantage of the exception handling below to rollback the quota, the same as if the instance.destroy() also failed with InstanceNotFound.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1708,"context_line":"                                instance.destroy()"},{"line_number":1709,"context_line":"                    else:"},{"line_number":1710,"context_line":"                        instance.destroy()"},{"line_number":1711,"context_line":"                else:"},{"line_number":1712,"context_line":"                    # The instance is already deleted so rollback the quota"},{"line_number":1713,"context_line":"                    # usage decrement reservation in the not found block below."},{"line_number":1714,"context_line":"                    raise exception.InstanceNotFound(instance_id\u003dinstance_uuid)"},{"line_number":1715,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1716,"context_line":"                pass"},{"line_number":1717,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_b5aecce4","line":1714,"range":{"start_line":1711,"start_character":16,"end_line":1714,"end_character":79},"in_reply_to":"3f1d235d_67b3f54e","updated":"2017-07-12 02:27:38.000000000","message":"Oh, derp. Yeah.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"        # Check quotas"},{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_caf890c5","line":2092,"range":{"start_line":2092,"start_character":22,"end_line":2092,"end_character":47},"updated":"2017-07-11 20:55:09.000000000","message":"Hmm, so is there any way we can slip over quota after we\u0027ve restored an instance? I\u0027m thinking of the recheck_quotas config option. In the before times, on soft delete we\u0027d decrement quota, right? Are we now double accounting for quota with a soft deleted and restored instance?\n\nSo if I was at usage\u003d10 and my limit\u003d10 (default), and I tried to restore a soft deleted instance, I think we\u0027ll fail with over quota here.\n\nTempest only has two tests for restoring a soft deleted instance and they are negative tests, so they don\u0027t actually test restore as a supported operation:\n\ntest_restore_server_invalid_state\n\ntest_restore_nonexistent_server_id\n\n\u003clater\u003e\n\nSince I haven\u0027t gotten to the count_as_dict function yet, I was concerned that we don\u0027t decrement usage during soft delete in the API which would then negatively affect the usage count when we restored, but the usage is counted here:\n\nhttps://review.openstack.org/#/c/416521/58/nova/quota.py@1892\n\nAnd deleted\u003dFalse should properly filter out soft deleted instances for the count:\n\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2167-L2192","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0cb595f39887de81c7e2536988cbe12ecb3a281b","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"        # Check quotas"},{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_98cb5aa4","line":2092,"range":{"start_line":2092,"start_character":22,"end_line":2092,"end_character":47},"in_reply_to":"1f1a1f67_7d88701e","updated":"2017-07-12 22:20:58.000000000","message":"We want to check quota (with a delta of 1) before we actually set the state. We\u0027re checking \"if we restore this one, will it fit in the quota limit?\" If yes, go ahead and set the state. Then it shows up in the count.\n\nA concern could be, two racing restores. Let\u0027s say quota limit in a project is 10 and there are 2 soft-deleted instances and 9 active instances. So usage is 9. If two users in the project restore 2 instances at the same time, both will pass the quota check (9 + 1 \u003c\u003d 10) and both will get restored. Now usage for the project is 11.\n\nTo guard against that, we would have to recheck quota with delta 0 after setting the state. And check fails, set the state back to soft-deleted and raise OverQuota.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a47f86970e3314c1963fe92695f8c7f1f98f173d","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"        # Check quotas"},{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_b8a5f6b7","line":2092,"range":{"start_line":2092,"start_character":22,"end_line":2092,"end_character":47},"in_reply_to":"1f1a1f67_98cb5aa4","updated":"2017-07-12 22:29:54.000000000","message":"Although, I now notice the state change here isn\u0027t the vm_state \u003d vm_states.SOFT_DELETED change. That doesn\u0027t happen until after an RPC cast. So to make strict quotas (recheck_quota) work here, we\u0027d have to set the vm_state here ahead of the RPC cast.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e958673a6d638745c8a78252d4d0e29a2dbebf22","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"        # Check quotas"},{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_f87c8eb6","line":2092,"range":{"start_line":2092,"start_character":22,"end_line":2092,"end_character":47},"in_reply_to":"1f1a1f67_b8a5f6b7","updated":"2017-07-12 22:52:58.000000000","message":"I\u0027m not sure I\u0027m following your last comment, the vm_state gets set to SOFT_DELETED when it\u0027s soft deleted, either in the API (if it\u0027s a local delete, which is dumb) or on the compute manager after the API casts to the manager to do the soft delete where it powers off the guest.\n\nThe vm_state is changed from SOFT_DELETED to ACTIVE on restore, either in the API here if there is no host, or after the RPC cast to the compute host.\n\nBut yeah, the instance counting function in the quota engine is filtering on deleted\u003dFalse, which does:\n\nhttps://github.com/openstack/nova/blob/6564a098bb35066974ccdb53fbabf0f7c3113c2a/nova/db/sqlalchemy/api.py#L2183-L2192\n\nSo when we count here, the vm_state is still SOFT_DELETED so it\u0027s not going to count b/c it\u0027ll be filtered out. And you\u0027re doing a delta\u003d1 count to see if we can change the state, which will then start accounting for this instance in the count of instances for the project - so yeah, I think I just said everything you said above, but I\u0027m making the connection now.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4952f25a1ebcea5527ebb33115f3379d76e339f8","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"        # Check quotas"},{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_b3cbd91c","line":2092,"range":{"start_line":2092,"start_character":22,"end_line":2092,"end_character":47},"in_reply_to":"1f1a1f67_f87c8eb6","updated":"2017-07-12 23:34:35.000000000","message":"Yeah, sorry, as mentioned on IRC got things completely backward in my head. I meant ACTIVE. So to handle a quota recheck, I think the only reasonable thing to do is add an extra filter to the instance count query to allow task_state.RESTORING to be counted, and then recheck based on after setting that.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fa3f2df8b58b23423b13087b2763099911cb9ac","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"        # Check quotas"},{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_7d88701e","line":2092,"range":{"start_line":2092,"start_character":22,"end_line":2092,"end_character":47},"in_reply_to":"3f1d235d_b54a4c24","updated":"2017-07-12 21:52:13.000000000","message":"\u003e When it\u0027s restored, the instance state won\u0027t be soft-deleted anymore\n\nWait, but does that happen before we count quota again? Looking below the state change doesn\u0027t happen until *after* we\u0027ve already counted quotas.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"        # Check quotas"},{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_b54a4c24","line":2092,"range":{"start_line":2092,"start_character":22,"end_line":2092,"end_character":47},"in_reply_to":"3f1d235d_caf890c5","updated":"2017-07-12 02:27:38.000000000","message":"Yeah, as discussed on IRC, as soon as the instance state goes to \"soft-deleted\" it won\u0027t show up in the count (so usage decremented). When it\u0027s restored, the instance state won\u0027t be soft-deleted anymore and it will show up in the count (usage incremented), so quota should be checked upon \"restore\" because it could go over.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"},{"line_number":2096,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_2aeca427","line":2093,"range":{"start_line":2093,"start_character":16,"end_line":2093,"end_character":54},"updated":"2017-07-11 20:55:09.000000000","message":"same as above, these are redundant since check_deltas will default to what\u0027s in the context, although check_num_instances_quota won\u0027t so we\u0027d have to account for that.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"},{"line_number":2096,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_d55ca8ee","line":2093,"range":{"start_line":2093,"start_character":16,"end_line":2093,"end_character":54},"in_reply_to":"3f1d235d_2aeca427","updated":"2017-07-12 02:27:38.000000000","message":"These are legit bc they\u0027re not from the context (they\u0027re from the ids_from_instance() call). They\u0027re from the already existing instance. Scenario: admin does a \"restore\" on non-admin\u0027s instance. Quota check should be scoped to the instance owner, not the admin.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fa3f2df8b58b23423b13087b2763099911cb9ac","unresolved":false,"context_lines":[{"line_number":2090,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2091,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2092,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"},{"line_number":2093,"context_line":"                project_id\u003dproject_id, user_id\u003duser_id)"},{"line_number":2094,"context_line":""},{"line_number":2095,"context_line":"        self._record_action_start(context, instance, instance_actions.RESTORE)"},{"line_number":2096,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_5d7db41a","line":2093,"range":{"start_line":2093,"start_character":16,"end_line":2093,"end_character":54},"in_reply_to":"3f1d235d_d55ca8ee","updated":"2017-07-12 21:52:13.000000000","message":"ah good call","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee9fa5442d520c7a6dba03845fb5faff4de3336d","unresolved":false,"context_lines":[{"line_number":3072,"context_line":"            deltas \u003d compute_utils.upsize_quota_delta(context,"},{"line_number":3073,"context_line":"                                                      new_instance_type,"},{"line_number":3074,"context_line":"                                                      current_instance_type)"},{"line_number":3075,"context_line":"            if deltas:"},{"line_number":3076,"context_line":"                try:"},{"line_number":3077,"context_line":"                    res_deltas \u003d {\u0027cores\u0027: deltas.get(\u0027cores\u0027, 0),"},{"line_number":3078,"context_line":"                                  \u0027ram\u0027: deltas.get(\u0027ram\u0027, 0)}"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_7a2191b4","line":3075,"range":{"start_line":3075,"start_character":12,"end_line":3075,"end_character":22},"updated":"2017-07-13 01:54:41.000000000","message":"OK this is here because this used to call reserve_quota_delta and that would check if deltas so you\u0027re just doing the same, got it.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":3139,"context_line":"        self.compute_task_api.resize_instance(context, instance,"},{"line_number":3140,"context_line":"                extra_instance_updates, scheduler_hint\u003dscheduler_hint,"},{"line_number":3141,"context_line":"                flavor\u003dnew_instance_type,"},{"line_number":3142,"context_line":"                reservations\u003d[],"},{"line_number":3143,"context_line":"                clean_shutdown\u003dclean_shutdown,"},{"line_number":3144,"context_line":"                request_spec\u003drequest_spec)"},{"line_number":3145,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_06dd9141","line":3142,"updated":"2017-07-13 15:22:42.000000000","message":"Reservations can be None here too as it is passed down to conductor/rpcapi::migrate_server() which defaults to None. I think it\u0027d be best to go ahead and snip this out of the conductor/api (or maybe default it to None) and remove the reservations\u003d param here entirely like above.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a19b0bf9fe6fdc9add5c1714c4bac7743b9d79b","unresolved":false,"context_lines":[{"line_number":3139,"context_line":"        self.compute_task_api.resize_instance(context, instance,"},{"line_number":3140,"context_line":"                extra_instance_updates, scheduler_hint\u003dscheduler_hint,"},{"line_number":3141,"context_line":"                flavor\u003dnew_instance_type,"},{"line_number":3142,"context_line":"                reservations\u003d[],"},{"line_number":3143,"context_line":"                clean_shutdown\u003dclean_shutdown,"},{"line_number":3144,"context_line":"                request_spec\u003drequest_spec)"},{"line_number":3145,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_9c00ce36","line":3142,"in_reply_to":"1f1a1f67_06dd9141","updated":"2017-07-13 16:04:05.000000000","message":"I would do that in a follow up or pre-patch rather than clutter up this already large and complicated change with any changes to the conductor task api resize_instance method signature. Could leave a TODO in here as a reminder to do that cleanup.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7afb366b9b3e27d72ac2f894c938b4f3258fd3df","unresolved":false,"context_lines":[{"line_number":982,"context_line":"        # In the case of any exceptions, attempt DB cleanup"},{"line_number":983,"context_line":"        except Exception:"},{"line_number":984,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":985,"context_line":"                for rs, br, im in instances_to_build:"},{"line_number":986,"context_line":"                    try:"},{"line_number":987,"context_line":"                        rs.destroy()"},{"line_number":988,"context_line":"                    except exception.RequestSpecNotFound:"},{"line_number":989,"context_line":"                        pass"},{"line_number":990,"context_line":"                    try:"},{"line_number":991,"context_line":"                        im.destroy()"},{"line_number":992,"context_line":"                    except exception.InstanceMappingNotFound:"},{"line_number":993,"context_line":"                        pass"},{"line_number":994,"context_line":"                    try:"},{"line_number":995,"context_line":"                        br.destroy()"},{"line_number":996,"context_line":"                    except exception.BuildRequestNotFound:"},{"line_number":997,"context_line":"                        pass"},{"line_number":998,"context_line":""},{"line_number":999,"context_line":"        return instances_to_build"},{"line_number":1000,"context_line":""}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_2674c8fd","line":997,"range":{"start_line":985,"start_character":16,"end_line":997,"end_character":28},"updated":"2017-07-16 14:27:49.000000000","message":"I\u0027m not totally sure why we handle the NotFound exceptions here, but maybe we should do that in your new method too?\n\nAlso, this is a nit, but you could re-use _cleanup_build_artifacts here by passing instances\u003d[] to that method so it doesn\u0027t do anything for instances but cleans up the reqspec/instancemapping/buildrequest.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9fb6fbfbd1eb1cf35c9f124071a7a117e4b745d4","unresolved":false,"context_lines":[{"line_number":982,"context_line":"        # In the case of any exceptions, attempt DB cleanup"},{"line_number":983,"context_line":"        except Exception:"},{"line_number":984,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":985,"context_line":"                for rs, br, im in instances_to_build:"},{"line_number":986,"context_line":"                    try:"},{"line_number":987,"context_line":"                        rs.destroy()"},{"line_number":988,"context_line":"                    except exception.RequestSpecNotFound:"},{"line_number":989,"context_line":"                        pass"},{"line_number":990,"context_line":"                    try:"},{"line_number":991,"context_line":"                        im.destroy()"},{"line_number":992,"context_line":"                    except exception.InstanceMappingNotFound:"},{"line_number":993,"context_line":"                        pass"},{"line_number":994,"context_line":"                    try:"},{"line_number":995,"context_line":"                        br.destroy()"},{"line_number":996,"context_line":"                    except exception.BuildRequestNotFound:"},{"line_number":997,"context_line":"                        pass"},{"line_number":998,"context_line":""},{"line_number":999,"context_line":"        return instances_to_build"},{"line_number":1000,"context_line":""}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_ca47374c","line":997,"range":{"start_line":985,"start_character":16,"end_line":997,"end_character":28},"in_reply_to":"1f1a1f67_2674c8fd","updated":"2017-07-17 19:42:43.000000000","message":"I think this is to handle races where the user has requested a delete of the instance before it\u0027s scheduled. In that situation, it\u0027s possible the build request was deleted by _delete_while_booting(). For the others, I\u0027m not sure. Maybe just insurance. I should probably do the same.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ff1a85f7a31c37f5a897d63927134dc5a95c1c3","unresolved":false,"context_lines":[{"line_number":982,"context_line":"        # In the case of any exceptions, attempt DB cleanup"},{"line_number":983,"context_line":"        except Exception:"},{"line_number":984,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":985,"context_line":"                for rs, br, im in instances_to_build:"},{"line_number":986,"context_line":"                    try:"},{"line_number":987,"context_line":"                        rs.destroy()"},{"line_number":988,"context_line":"                    except exception.RequestSpecNotFound:"},{"line_number":989,"context_line":"                        pass"},{"line_number":990,"context_line":"                    try:"},{"line_number":991,"context_line":"                        im.destroy()"},{"line_number":992,"context_line":"                    except exception.InstanceMappingNotFound:"},{"line_number":993,"context_line":"                        pass"},{"line_number":994,"context_line":"                    try:"},{"line_number":995,"context_line":"                        br.destroy()"},{"line_number":996,"context_line":"                    except exception.BuildRequestNotFound:"},{"line_number":997,"context_line":"                        pass"},{"line_number":998,"context_line":""},{"line_number":999,"context_line":"        return instances_to_build"},{"line_number":1000,"context_line":""}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_53319b59","line":997,"range":{"start_line":985,"start_character":16,"end_line":997,"end_character":28},"in_reply_to":"1f1a1f67_ca47374c","updated":"2017-07-18 02:02:07.000000000","message":"Ah yeah, good point, forgot about delete while booting.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2feeaa70ab1001b89db8a13a2f195791b2a1d33","unresolved":false,"context_lines":[{"line_number":1161,"context_line":"                        # Need to clean up all the instances we created"},{"line_number":1162,"context_line":"                        # along with the build requests, request specs,"},{"line_number":1163,"context_line":"                        # and instance mappings."},{"line_number":1164,"context_line":"                        self._cleanup_build_artifacts(instances,"},{"line_number":1165,"context_line":"                                                      instances_to_build)"},{"line_number":1166,"context_line":""},{"line_number":1167,"context_line":"            self.compute_task_api.build_instances(context,"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_269cc868","line":1164,"range":{"start_line":1164,"start_character":29,"end_line":1164,"end_character":53},"updated":"2017-07-16 14:05:03.000000000","message":"In earlier reviews Dan was talking about some weird UX where the user could be listing instances while building and we can\u0027t delete them, but at this point we haven\u0027t cast to conductor so the user hasn\u0027t gotten a response, so doing the cleanup here in the API is OK - just making a note to myself. It\u0027s once we get to conductor that we have to not delete the instances but put them into ERROR state.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2feeaa70ab1001b89db8a13a2f195791b2a1d33","unresolved":false,"context_lines":[{"line_number":2095,"context_line":"    def restore(self, context, instance):"},{"line_number":2096,"context_line":"        \"\"\"Restore a previously deleted (but not reclaimed) instance.\"\"\""},{"line_number":2097,"context_line":"        # Check quotas"},{"line_number":2098,"context_line":"        # TODO(melwitt): We\u0027re not rechecking for strict quota here to guard"},{"line_number":2099,"context_line":"        # against going over quota during a race at this time because the"},{"line_number":2100,"context_line":"        # resource consumption for this operation is written to the database"},{"line_number":2101,"context_line":"        # by compute."},{"line_number":2102,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2103,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2104,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_8686fc12","line":2101,"range":{"start_line":2098,"start_character":8,"end_line":2101,"end_character":21},"updated":"2017-07-16 14:05:03.000000000","message":"In an earlier reply you said,\n\n\u003e So to handle a quota recheck, I think the only reasonable thing to do is add an extra filter to the instance count query to allow task_state.RESTORING to be counted, and then recheck based on after setting that.\n\nIs the TODO just to explore that later so we don\u0027t hold up this change?\n\nNote that it\u0027s not necessarily checked on the compute if the instance.host is None. Couldn\u0027t we recheck in the local restore case?\n\nI think we need some functional testing around soft delete/restore anyway since we don\u0027t test it in Tempest, so we could spend some time working on that later and then once we have the functional testing in place we could work on rechecks for quota.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"76fe8d6cec4d392dbaf7aac132fae82905e7e33c","unresolved":false,"context_lines":[{"line_number":2095,"context_line":"    def restore(self, context, instance):"},{"line_number":2096,"context_line":"        \"\"\"Restore a previously deleted (but not reclaimed) instance.\"\"\""},{"line_number":2097,"context_line":"        # Check quotas"},{"line_number":2098,"context_line":"        # TODO(melwitt): We\u0027re not rechecking for strict quota here to guard"},{"line_number":2099,"context_line":"        # against going over quota during a race at this time because the"},{"line_number":2100,"context_line":"        # resource consumption for this operation is written to the database"},{"line_number":2101,"context_line":"        # by compute."},{"line_number":2102,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2103,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2104,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_e7d8eeef","line":2101,"range":{"start_line":2098,"start_character":8,"end_line":2101,"end_character":21},"in_reply_to":"1f1a1f67_8686fc12","updated":"2017-07-17 19:36:56.000000000","message":"Yeah, I looked at adding vm_state.SOFT_DELETED + task_state.RESTORING instances to the count and I think it would require a WHERE clause something like: \n\n WHERE (deleted\u003d0 AND etc) OR (vm_state\u003dSOFT_DELETED AND task_state\u003dRESTORING)\n\nand I wasn\u0027t sure if it was worth doing right now just to protect against restore races. We don\u0027t think many/any users use soft-delete/restore. So the TODO was to not forget about revisiting it and deciding whether it\u0027s worth it.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ff1a85f7a31c37f5a897d63927134dc5a95c1c3","unresolved":false,"context_lines":[{"line_number":2095,"context_line":"    def restore(self, context, instance):"},{"line_number":2096,"context_line":"        \"\"\"Restore a previously deleted (but not reclaimed) instance.\"\"\""},{"line_number":2097,"context_line":"        # Check quotas"},{"line_number":2098,"context_line":"        # TODO(melwitt): We\u0027re not rechecking for strict quota here to guard"},{"line_number":2099,"context_line":"        # against going over quota during a race at this time because the"},{"line_number":2100,"context_line":"        # resource consumption for this operation is written to the database"},{"line_number":2101,"context_line":"        # by compute."},{"line_number":2102,"context_line":"        flavor \u003d instance.get_flavor()"},{"line_number":2103,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context, instance)"},{"line_number":2104,"context_line":"        compute_utils.check_num_instances_quota(context, flavor, 1, 1,"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_531abbcb","line":2101,"range":{"start_line":2098,"start_character":8,"end_line":2101,"end_character":21},"in_reply_to":"1f1a1f67_e7d8eeef","updated":"2017-07-18 02:02:07.000000000","message":"OK, I think I\u0027m fine with leaving a TODO for now and dealing with that later when we have some better functional testing in place first.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2feeaa70ab1001b89db8a13a2f195791b2a1d33","unresolved":false,"context_lines":[{"line_number":2962,"context_line":"    def _check_quota_for_upsize(context, instance, current_flavor, new_flavor):"},{"line_number":2963,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context,"},{"line_number":2964,"context_line":"                                                           instance)"},{"line_number":2965,"context_line":"        deltas \u003d compute_utils.upsize_quota_delta(context, new_flavor,"},{"line_number":2966,"context_line":"                                                  current_flavor)"},{"line_number":2967,"context_line":"        if deltas:"},{"line_number":2968,"context_line":"            try:"},{"line_number":2969,"context_line":"                res_deltas \u003d {\u0027cores\u0027: deltas.get(\u0027cores\u0027, 0),"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_0672ec49","line":2966,"range":{"start_line":2965,"start_character":8,"end_line":2966,"end_character":65},"updated":"2017-07-16 14:05:03.000000000","message":"Just for my own sanity, it could be nice to leave a comment here that deltas will be empty if the resize is downsizing the instance flavor. The utils code around this isn\u0027t super obvious.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"76fe8d6cec4d392dbaf7aac132fae82905e7e33c","unresolved":false,"context_lines":[{"line_number":2962,"context_line":"    def _check_quota_for_upsize(context, instance, current_flavor, new_flavor):"},{"line_number":2963,"context_line":"        project_id, user_id \u003d quotas_obj.ids_from_instance(context,"},{"line_number":2964,"context_line":"                                                           instance)"},{"line_number":2965,"context_line":"        deltas \u003d compute_utils.upsize_quota_delta(context, new_flavor,"},{"line_number":2966,"context_line":"                                                  current_flavor)"},{"line_number":2967,"context_line":"        if deltas:"},{"line_number":2968,"context_line":"            try:"},{"line_number":2969,"context_line":"                res_deltas \u003d {\u0027cores\u0027: deltas.get(\u0027cores\u0027, 0),"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_67f61e66","line":2966,"range":{"start_line":2965,"start_character":8,"end_line":2966,"end_character":65},"in_reply_to":"1f1a1f67_0672ec49","updated":"2017-07-17 19:36:56.000000000","message":"Can do.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2feeaa70ab1001b89db8a13a2f195791b2a1d33","unresolved":false,"context_lines":[{"line_number":3024,"context_line":"    @check_instance_lock"},{"line_number":3025,"context_line":"    @check_instance_cell"},{"line_number":3026,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.RESIZED])"},{"line_number":3027,"context_line":"    def confirm_resize(self, context, instance, migration\u003dNone):"},{"line_number":3028,"context_line":"        \"\"\"Confirms a migration/resize and deletes the \u0027old\u0027 instance.\"\"\""},{"line_number":3029,"context_line":"        elevated \u003d context.elevated()"},{"line_number":3030,"context_line":"        if migration is None:"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_c6951435","line":3027,"range":{"start_line":3027,"start_character":8,"end_line":3027,"end_character":22},"updated":"2017-07-16 14:05:03.000000000","message":"Didn\u0027t we talk about why we don\u0027t check quotas again on confirm? I can\u0027t exactly remember what we said but it might be good to have a comment in here explaining why we don\u0027t check quota again on confirm. We check on the initial resize, and let\u0027s say I pass the check then, but then before I confirm, I create another instance which maxes out my ram usage and now when I confirm I\u0027m going to go over quota due to ram usage on the new flavor for the resize.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ff1a85f7a31c37f5a897d63927134dc5a95c1c3","unresolved":false,"context_lines":[{"line_number":3024,"context_line":"    @check_instance_lock"},{"line_number":3025,"context_line":"    @check_instance_cell"},{"line_number":3026,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.RESIZED])"},{"line_number":3027,"context_line":"    def confirm_resize(self, context, instance, migration\u003dNone):"},{"line_number":3028,"context_line":"        \"\"\"Confirms a migration/resize and deletes the \u0027old\u0027 instance.\"\"\""},{"line_number":3029,"context_line":"        elevated \u003d context.elevated()"},{"line_number":3030,"context_line":"        if migration is None:"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_9353b339","line":3027,"range":{"start_line":3027,"start_character":8,"end_line":3027,"end_character":22},"in_reply_to":"1f1a1f67_27482685","updated":"2017-07-18 02:02:07.000000000","message":"\u003e Usage changes after the user requests a resize after compute changes the instance state (changes the vcpus and memory_mb).\n\nOK that\u0027s what I\u0027m seeing here then:\n\nhttps://review.openstack.org/#/c/484152/\n\nBecause before counting quotas, resize-down usage wouldn\u0027t change until you confirmed it (because of the reservation). Now usage changes as soon as the compute adjusts the vcpus and ram count on the instance. That\u0027s not really any API contract we\u0027ve ever communicated, so I\u0027m not terribly worried about it. And I don\u0027t think my test would even be able to tell if I wasn\u0027t including \u0027reserved\u0027 amounts in the limits API call.\n\n\u003e The only reason to recheck here is if we wanted to have a late check on a resize race. Meaning, if two parallel resize ups raced and went over quota after compute updated the state, then confirm or revert would be the only opportunity to catch someone over quota and force the first one who tries to confirm, to revert (by blocking their confirm).\n\nI know some people/products (like PowerVC) sets the auto-confirm option to 1 second so it\u0027s an immediate confirm basically from the user point of view. I believe Tempest also does that once it goes into RESIZE_VERIFY state, but those Tempest tests are single-tenant on a single instance, so they wouldn\u0027t tickle that kind of issue.\n\nI don\u0027t want to bend over backward for this in this change, but we should maybe leave NOTE explaining why we\u0027re not checking quota again here, but it could be done in the future if needed.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"76fe8d6cec4d392dbaf7aac132fae82905e7e33c","unresolved":false,"context_lines":[{"line_number":3024,"context_line":"    @check_instance_lock"},{"line_number":3025,"context_line":"    @check_instance_cell"},{"line_number":3026,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.RESIZED])"},{"line_number":3027,"context_line":"    def confirm_resize(self, context, instance, migration\u003dNone):"},{"line_number":3028,"context_line":"        \"\"\"Confirms a migration/resize and deletes the \u0027old\u0027 instance.\"\"\""},{"line_number":3029,"context_line":"        elevated \u003d context.elevated()"},{"line_number":3030,"context_line":"        if migration is None:"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_27482685","line":3027,"range":{"start_line":3027,"start_character":8,"end_line":3027,"end_character":22},"in_reply_to":"1f1a1f67_86d39cd5","updated":"2017-07-17 19:36:56.000000000","message":"My thinking on why not to recheck on confirm was that a confirm doesn\u0027t result in a usage change at all. If you confirm a resize down, usage doesn\u0027t change. If you confirm a resize up, usage doesn\u0027t change. Usage changes after the user requests a resize after compute changes the instance state (changes the vcpus and memory_mb).\n\nThe only reason to recheck here is if we wanted to have a late check on a resize race. Meaning, if two parallel resize ups raced and went over quota after compute updated the state, then confirm or revert would be the only opportunity to catch someone over quota and force the first one who tries to confirm, to revert (by blocking their confirm).\n\nDo you think we should check here to do as described in the second paragraph?","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"647a9f5bccc6c836c5add26a1870019a8fba6c42","unresolved":false,"context_lines":[{"line_number":3024,"context_line":"    @check_instance_lock"},{"line_number":3025,"context_line":"    @check_instance_cell"},{"line_number":3026,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.RESIZED])"},{"line_number":3027,"context_line":"    def confirm_resize(self, context, instance, migration\u003dNone):"},{"line_number":3028,"context_line":"        \"\"\"Confirms a migration/resize and deletes the \u0027old\u0027 instance.\"\"\""},{"line_number":3029,"context_line":"        elevated \u003d context.elevated()"},{"line_number":3030,"context_line":"        if migration is None:"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_69002802","line":3027,"range":{"start_line":3027,"start_character":8,"end_line":3027,"end_character":22},"in_reply_to":"1f1a1f67_9353b339","updated":"2017-07-18 02:43:24.000000000","message":"Yeah, okay. If you\u0027re talking about what you see in the \"used\" part of the used limits API, then agreed there\u0027s a behavior change. What I was saying is that as far as checking quota (checking usage against limits) the behavior should stay the same as before counting quotas because the limit check takes into account reserved + used.\n\nAnd sure, I\u0027ll add a NOTE explaining why we\u0027re not checking quota here and if we change our mind we could.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7afb366b9b3e27d72ac2f894c938b4f3258fd3df","unresolved":false,"context_lines":[{"line_number":3024,"context_line":"    @check_instance_lock"},{"line_number":3025,"context_line":"    @check_instance_cell"},{"line_number":3026,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.RESIZED])"},{"line_number":3027,"context_line":"    def confirm_resize(self, context, instance, migration\u003dNone):"},{"line_number":3028,"context_line":"        \"\"\"Confirms a migration/resize and deletes the \u0027old\u0027 instance.\"\"\""},{"line_number":3029,"context_line":"        elevated \u003d context.elevated()"},{"line_number":3030,"context_line":"        if migration is None:"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_86d39cd5","line":3027,"range":{"start_line":3027,"start_character":8,"end_line":3027,"end_character":22},"in_reply_to":"1f1a1f67_c6951435","updated":"2017-07-16 14:27:49.000000000","message":"I think we said it would be a good idea to check quota on confirm for an upsize, but not for a downsize. But my memory is hazy since we\u0027ve talked about a lot of things.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        for instance in instances or []:"},{"line_number":1190,"context_line":"            try:"},{"line_number":1191,"context_line":"                instance.destroy()"},{"line_number":1192,"context_line":"            except exception.InstanceNotFound:"},{"line_number":1193,"context_line":"                pass"},{"line_number":1194,"context_line":"        for rs, build_request, im in instances_to_build or []:"},{"line_number":1195,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_91470278","line":1192,"updated":"2017-07-19 01:58:18.000000000","message":"The coverage report tells me that we don\u0027t have any unit tests that make sure we handle the various NotFound exceptions in here. That could be added in a separate follow up change.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":2148,"context_line":"            # database by compute."},{"line_number":2149,"context_line":"            self.compute_rpcapi.restore_instance(context, instance)"},{"line_number":2150,"context_line":"        else:"},{"line_number":2151,"context_line":"            instance.vm_state \u003d vm_states.ACTIVE"},{"line_number":2152,"context_line":"            instance.task_state \u003d None"},{"line_number":2153,"context_line":"            instance.deleted_at \u003d None"},{"line_number":2154,"context_line":"            instance.save(expected_task_state\u003d[None])"},{"line_number":2155,"context_line":""},{"line_number":2156,"context_line":"    @check_instance_lock"},{"line_number":2157,"context_line":"    @check_instance_state(must_have_launched\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_51228aba","line":2154,"range":{"start_line":2151,"start_character":12,"end_line":2154,"end_character":53},"updated":"2017-07-19 01:58:18.000000000","message":"Apparently we have no unit test coverage for this case, but that\u0027s latent and not introduced by this change.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":3067,"context_line":"        # NOTE(melwitt): We\u0027re not checking quota here because there isn\u0027t a"},{"line_number":3068,"context_line":"        # change in resource usage when confirming a resize. Resource"},{"line_number":3069,"context_line":"        # consumption for resizes are written to the database by compute, so"},{"line_number":3070,"context_line":"        # a confirm resize is just a clean up of the migration objects and a"},{"line_number":3071,"context_line":"        # state change in compute."},{"line_number":3072,"context_line":"        if migration is None:"},{"line_number":3073,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":3074,"context_line":"                elevated, instance.uuid, \u0027finished\u0027)"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_31470ebf","line":3071,"range":{"start_line":3070,"start_character":71,"end_line":3071,"end_character":34},"updated":"2017-07-19 01:58:18.000000000","message":"There is more to what happens in compute, but meh.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f4c3e917e5d9746083efa5dc04d2f08679a7dfc0","unresolved":false,"context_lines":[{"line_number":3067,"context_line":"        # NOTE(melwitt): We\u0027re not checking quota here because there isn\u0027t a"},{"line_number":3068,"context_line":"        # change in resource usage when confirming a resize. Resource"},{"line_number":3069,"context_line":"        # consumption for resizes are written to the database by compute, so"},{"line_number":3070,"context_line":"        # a confirm resize is just a clean up of the migration objects and a"},{"line_number":3071,"context_line":"        # state change in compute."},{"line_number":3072,"context_line":"        if migration is None:"},{"line_number":3073,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":3074,"context_line":"                elevated, instance.uuid, \u0027finished\u0027)"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_409df107","line":3071,"range":{"start_line":3070,"start_character":71,"end_line":3071,"end_character":34},"in_reply_to":"1f1a1f67_31470ebf","updated":"2017-07-19 22:49:41.000000000","message":"I didn\u0027t see more than removal of migration-related objects and a state change to the instance in confirm_resize in compute, so maybe I missed something.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee9fa5442d520c7a6dba03845fb5faff4de3336d","unresolved":false,"context_lines":[{"line_number":701,"context_line":"                                                               instance)"},{"line_number":702,"context_line":"        quotas.reserve(project_id\u003dproject_id, user_id\u003duser_id, instances\u003d-1,"},{"line_number":703,"context_line":"                       cores\u003d-instance.flavor.vcpus,"},{"line_number":704,"context_line":"                       ram\u003d-instance.flavor.memory_mb)"},{"line_number":705,"context_line":"        self._complete_deletion(context,"},{"line_number":706,"context_line":"                                instance,"},{"line_number":707,"context_line":"                                bdms,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_1a1a9d7c","side":"PARENT","line":704,"range":{"start_line":704,"start_character":23,"end_line":704,"end_character":53},"updated":"2017-07-13 01:54:41.000000000","message":"Oh yay another place where we weren\u0027t accounting for decrementing based on video ram usage. :)","commit_id":"dcea3ff68793a28cd546805ebcb2c0ae6c9b684d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee9fa5442d520c7a6dba03845fb5faff4de3336d","unresolved":false,"context_lines":[{"line_number":2099,"context_line":"                if network_info is not None:"},{"line_number":2100,"context_line":"                    network_info.wait(do_raise\u003dFalse)"},{"line_number":2101,"context_line":"        except (exception.UnexpectedTaskStateError,"},{"line_number":2102,"context_line":"                exception.OverQuota, exception.InvalidBDM) as e:"},{"line_number":2103,"context_line":"            # Make sure the async call finishes"},{"line_number":2104,"context_line":"            if network_info is not None:"},{"line_number":2105,"context_line":"                network_info.wait(do_raise\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_1d05f7c2","line":2102,"range":{"start_line":2102,"start_character":26,"end_line":2102,"end_character":35},"updated":"2017-07-13 01:54:41.000000000","message":"OK this comes from _prep_block_device if cinder gives us an over quota error when creating a volume.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee9fa5442d520c7a6dba03845fb5faff4de3336d","unresolved":false,"context_lines":[{"line_number":3785,"context_line":"        try:"},{"line_number":3786,"context_line":"            reschedule_method \u003d self.compute_task_api.resize_instance"},{"line_number":3787,"context_line":"            scheduler_hint \u003d dict(filter_properties\u003dfilter_properties)"},{"line_number":3788,"context_line":"            method_args \u003d (instance, None, scheduler_hint, instance_type)"},{"line_number":3789,"context_line":"            task_state \u003d task_states.RESIZE_PREP"},{"line_number":3790,"context_line":""},{"line_number":3791,"context_line":"            rescheduled \u003d self._reschedule(context, request_spec,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_fd3b5b99","line":3788,"range":{"start_line":3788,"start_character":12,"end_line":3788,"end_character":73},"updated":"2017-07-13 01:54:41.000000000","message":"I think this is wrong, you have to pass None or [] for reservations because it\u0027s not a kwarg:\n\nhttps://github.com/openstack/nova/blob/61551786a8ce1628dcf73faf7f3ac514c99587af/nova/conductor/api.py#L88","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2a19b0bf9fe6fdc9add5c1714c4bac7743b9d79b","unresolved":false,"context_lines":[{"line_number":3785,"context_line":"        try:"},{"line_number":3786,"context_line":"            reschedule_method \u003d self.compute_task_api.resize_instance"},{"line_number":3787,"context_line":"            scheduler_hint \u003d dict(filter_properties\u003dfilter_properties)"},{"line_number":3788,"context_line":"            method_args \u003d (instance, None, scheduler_hint, instance_type)"},{"line_number":3789,"context_line":"            task_state \u003d task_states.RESIZE_PREP"},{"line_number":3790,"context_line":""},{"line_number":3791,"context_line":"            rescheduled \u003d self._reschedule(context, request_spec,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_5cc23635","line":3788,"range":{"start_line":3788,"start_character":12,"end_line":3788,"end_character":73},"in_reply_to":"1f1a1f67_26bd2db7","updated":"2017-07-13 16:04:05.000000000","message":"\u003e Yeah, how is this working?\n\nIt\u0027s a reschedule which we probably don\u0027t actually hit in Tempest testing since we have a single compute host in most CI runs, so we just don\u0027t test this code beyond unit tests, and I\u0027m sure the unit tests are mocking everything out.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":3785,"context_line":"        try:"},{"line_number":3786,"context_line":"            reschedule_method \u003d self.compute_task_api.resize_instance"},{"line_number":3787,"context_line":"            scheduler_hint \u003d dict(filter_properties\u003dfilter_properties)"},{"line_number":3788,"context_line":"            method_args \u003d (instance, None, scheduler_hint, instance_type)"},{"line_number":3789,"context_line":"            task_state \u003d task_states.RESIZE_PREP"},{"line_number":3790,"context_line":""},{"line_number":3791,"context_line":"            rescheduled \u003d self._reschedule(context, request_spec,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_2a0e0732","line":3788,"range":{"start_line":3788,"start_character":12,"end_line":3788,"end_character":73},"in_reply_to":"1f1a1f67_26bd2db7","updated":"2017-07-13 19:10:07.000000000","message":"Okay, so just add defaults to None in a prior patch and then remove all the passing of reservations in here.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":3785,"context_line":"        try:"},{"line_number":3786,"context_line":"            reschedule_method \u003d self.compute_task_api.resize_instance"},{"line_number":3787,"context_line":"            scheduler_hint \u003d dict(filter_properties\u003dfilter_properties)"},{"line_number":3788,"context_line":"            method_args \u003d (instance, None, scheduler_hint, instance_type)"},{"line_number":3789,"context_line":"            task_state \u003d task_states.RESIZE_PREP"},{"line_number":3790,"context_line":""},{"line_number":3791,"context_line":"            rescheduled \u003d self._reschedule(context, request_spec,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_26bd2db7","line":3788,"range":{"start_line":3788,"start_character":12,"end_line":3788,"end_character":73},"in_reply_to":"1f1a1f67_fd3b5b99","updated":"2017-07-13 15:22:42.000000000","message":"Yeah, how is this working?\n\nAlso, I think we should either remove it or default it to None for removal later like I said previously. No reason to have a random None in here we don\u0027t realize should be gone in a year.\n\nIt\u0027d be super easy to slide a change in front of this that defaults all those things to None and then just remove reservations from the calls in this patch and then circle back later with actual removal from the signature(s). That would avoid making this any larger and would still make the debt less.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ab75d9d74b142e58e7876484a81a56500a88fe6f","unresolved":false,"context_lines":[{"line_number":3903,"context_line":"    @staticmethod"},{"line_number":3904,"context_line":"    def _set_instance_info(instance, instance_type):"},{"line_number":3905,"context_line":"        instance.instance_type_id \u003d instance_type.id"},{"line_number":3906,"context_line":"        # NOTE(danms): These are purely for any legacy code that still"},{"line_number":3907,"context_line":"        # looks at them."},{"line_number":3908,"context_line":"        instance.memory_mb \u003d instance_type.memory_mb"},{"line_number":3909,"context_line":"        instance.vcpus \u003d instance_type.vcpus"},{"line_number":3910,"context_line":"        instance.root_gb \u003d instance_type.root_gb"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_f0f8fe49","line":3907,"range":{"start_line":3906,"start_character":8,"end_line":3907,"end_character":24},"updated":"2017-07-13 20:35:36.000000000","message":"We should remove this comment since the ram/cores counting is going to be based on these values in the instance now.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"}],"nova/compute/utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":707,"context_line":"    return headroom"},{"line_number":708,"context_line":""},{"line_number":709,"context_line":""},{"line_number":710,"context_line":"def check_num_instances_quota(context, instance_type, min_count,"},{"line_number":711,"context_line":"                              max_count, project_id\u003dNone, user_id\u003dNone):"},{"line_number":712,"context_line":"    \"\"\"Enforce quota limits on number of instances created.\"\"\""},{"line_number":713,"context_line":""},{"line_number":714,"context_line":"    # Determine requested cores and ram"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_c7e64917","line":711,"range":{"start_line":710,"start_character":54,"end_line":711,"end_character":39},"updated":"2017-07-11 20:55:09.000000000","message":"Just a note to myself, these will be 0 when doing a recheck_quota operation after the instance is created. Need to be aware of any weird cases where 0 will break something, like divide by 0 etc.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":713,"context_line":""},{"line_number":714,"context_line":"    # Determine requested cores and ram"},{"line_number":715,"context_line":"    req_cores \u003d max_count * instance_type[\u0027vcpus\u0027]"},{"line_number":716,"context_line":"    vram_mb \u003d int(instance_type.get(\u0027extra_specs\u0027, {}).get(VIDEO_RAM, 0))"},{"line_number":717,"context_line":"    req_ram \u003d max_count * (instance_type[\u0027memory_mb\u0027] + vram_mb)"},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_ca9b13c7","line":716,"updated":"2017-07-13 19:10:07.000000000","message":"Need to remove this.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":723,"context_line":"                                    project_id, user_id\u003duser_id,"},{"line_number":724,"context_line":"                                    check_project_id\u003dproject_id,"},{"line_number":725,"context_line":"                                    check_user_id\u003duser_id)"},{"line_number":726,"context_line":"    except exception.OverQuota as exc:"},{"line_number":727,"context_line":"        # OK, we exceeded quota; let\u0027s figure out why..."},{"line_number":728,"context_line":"        quotas \u003d exc.kwargs[\u0027quotas\u0027]"},{"line_number":729,"context_line":"        overs \u003d exc.kwargs[\u0027overs\u0027]"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_671c15c1","line":726,"range":{"start_line":726,"start_character":4,"end_line":726,"end_character":38},"updated":"2017-07-11 20:55:09.000000000","message":"If we go over quota because of a recheck quota where min/max are 0, do we need to do any of the rest below? As noted below, things get a bit weird when min and max is 0, at least with the error message.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4952f25a1ebcea5527ebb33115f3379d76e339f8","unresolved":false,"context_lines":[{"line_number":723,"context_line":"                                    project_id, user_id\u003duser_id,"},{"line_number":724,"context_line":"                                    check_project_id\u003dproject_id,"},{"line_number":725,"context_line":"                                    check_user_id\u003duser_id)"},{"line_number":726,"context_line":"    except exception.OverQuota as exc:"},{"line_number":727,"context_line":"        # OK, we exceeded quota; let\u0027s figure out why..."},{"line_number":728,"context_line":"        quotas \u003d exc.kwargs[\u0027quotas\u0027]"},{"line_number":729,"context_line":"        overs \u003d exc.kwargs[\u0027overs\u0027]"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_784abe80","line":726,"range":{"start_line":726,"start_character":4,"end_line":726,"end_character":38},"in_reply_to":"1f1a1f67_dd4b0485","updated":"2017-07-12 23:34:35.000000000","message":"Guh, I missed the point \"min/max 0\" yeah, I think we can.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":723,"context_line":"                                    project_id, user_id\u003duser_id,"},{"line_number":724,"context_line":"                                    check_project_id\u003dproject_id,"},{"line_number":725,"context_line":"                                    check_user_id\u003duser_id)"},{"line_number":726,"context_line":"    except exception.OverQuota as exc:"},{"line_number":727,"context_line":"        # OK, we exceeded quota; let\u0027s figure out why..."},{"line_number":728,"context_line":"        quotas \u003d exc.kwargs[\u0027quotas\u0027]"},{"line_number":729,"context_line":"        overs \u003d exc.kwargs[\u0027overs\u0027]"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_95c63049","line":726,"range":{"start_line":726,"start_character":4,"end_line":726,"end_character":38},"in_reply_to":"3f1d235d_671c15c1","updated":"2017-07-12 02:27:38.000000000","message":"No, this is the logic that calculates how many instances will fit in the remaining quota if max_count won\u0027t fit. And if that is at least min_count, it will go and try to create N instances that will fit.\n\nI don\u0027t think anything ever divides by the delta value, but that\u0027s a good thing to look out for.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5606207ded920bfb422b28c78a850fa88a7747ed","unresolved":false,"context_lines":[{"line_number":723,"context_line":"                                    project_id, user_id\u003duser_id,"},{"line_number":724,"context_line":"                                    check_project_id\u003dproject_id,"},{"line_number":725,"context_line":"                                    check_user_id\u003duser_id)"},{"line_number":726,"context_line":"    except exception.OverQuota as exc:"},{"line_number":727,"context_line":"        # OK, we exceeded quota; let\u0027s figure out why..."},{"line_number":728,"context_line":"        quotas \u003d exc.kwargs[\u0027quotas\u0027]"},{"line_number":729,"context_line":"        overs \u003d exc.kwargs[\u0027overs\u0027]"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_dd4b0485","line":726,"range":{"start_line":726,"start_character":4,"end_line":726,"end_character":38},"in_reply_to":"3f1d235d_95c63049","updated":"2017-07-12 22:42:16.000000000","message":"Right, but I mean, if we get here and min_count and max_count are 0, should we just reraise instead of continuing with the below stuff?","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":736,"context_line":"        # Reduce \u0027allowed\u0027 instances in line with the cores \u0026 ram headroom"},{"line_number":737,"context_line":"        if instance_type[\u0027vcpus\u0027]:"},{"line_number":738,"context_line":"            allowed \u003d min(allowed,"},{"line_number":739,"context_line":"                          headroom[\u0027cores\u0027] // instance_type[\u0027vcpus\u0027])"},{"line_number":740,"context_line":"        if instance_type[\u0027memory_mb\u0027]:"},{"line_number":741,"context_line":"            allowed \u003d min(allowed,"},{"line_number":742,"context_line":"                          headroom[\u0027ram\u0027] // (instance_type[\u0027memory_mb\u0027] +"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_86abe11b","line":739,"range":{"start_line":739,"start_character":47,"end_line":739,"end_character":69},"updated":"2017-07-13 15:22:42.000000000","message":"These are flavor objects and can/should use dot notation right?","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6b417e48c94ed2af84292de5a70caa8625270d1a","unresolved":false,"context_lines":[{"line_number":736,"context_line":"        # Reduce \u0027allowed\u0027 instances in line with the cores \u0026 ram headroom"},{"line_number":737,"context_line":"        if instance_type[\u0027vcpus\u0027]:"},{"line_number":738,"context_line":"            allowed \u003d min(allowed,"},{"line_number":739,"context_line":"                          headroom[\u0027cores\u0027] // instance_type[\u0027vcpus\u0027])"},{"line_number":740,"context_line":"        if instance_type[\u0027memory_mb\u0027]:"},{"line_number":741,"context_line":"            allowed \u003d min(allowed,"},{"line_number":742,"context_line":"                          headroom[\u0027ram\u0027] // (instance_type[\u0027memory_mb\u0027] +"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_6a73dfb2","line":739,"range":{"start_line":739,"start_character":47,"end_line":739,"end_character":69},"in_reply_to":"1f1a1f67_86abe11b","updated":"2017-07-13 19:10:07.000000000","message":"Yeah, looks like it is now. I just moved this code and didn\u0027t pay attention to changing it.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        # Convert to the appropriate exception message"},{"line_number":746,"context_line":"        if allowed \u003c\u003d 0:"},{"line_number":747,"context_line":"            msg \u003d \"Cannot run any more instances of this type.\""},{"line_number":748,"context_line":"        elif min_count \u003c\u003d allowed \u003c\u003d max_count:"},{"line_number":749,"context_line":"            # We\u0027re actually OK, but still need to check against allowed"},{"line_number":750,"context_line":"            return check_num_instances_quota(context, instance_type, min_count,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_15a8ead8","line":747,"range":{"start_line":747,"start_character":18,"end_line":747,"end_character":63},"updated":"2017-07-11 20:55:09.000000000","message":"You dropped the translation on these msg variables, but they only get logged at debug so removing the translation marker is fine.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        # Convert to the appropriate exception message"},{"line_number":746,"context_line":"        if allowed \u003c\u003d 0:"},{"line_number":747,"context_line":"            msg \u003d \"Cannot run any more instances of this type.\""},{"line_number":748,"context_line":"        elif min_count \u003c\u003d allowed \u003c\u003d max_count:"},{"line_number":749,"context_line":"            # We\u0027re actually OK, but still need to check against allowed"},{"line_number":750,"context_line":"            return check_num_instances_quota(context, instance_type, min_count,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_35b87cc6","line":747,"range":{"start_line":747,"start_character":18,"end_line":747,"end_character":63},"in_reply_to":"3f1d235d_15a8ead8","updated":"2017-07-12 02:27:38.000000000","message":"I got dinged on some other patches for having LW_ etc so I thought I was supposed to not add any translation stuff to anything.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5606207ded920bfb422b28c78a850fa88a7747ed","unresolved":false,"context_lines":[{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        # Convert to the appropriate exception message"},{"line_number":746,"context_line":"        if allowed \u003c\u003d 0:"},{"line_number":747,"context_line":"            msg \u003d \"Cannot run any more instances of this type.\""},{"line_number":748,"context_line":"        elif min_count \u003c\u003d allowed \u003c\u003d max_count:"},{"line_number":749,"context_line":"            # We\u0027re actually OK, but still need to check against allowed"},{"line_number":750,"context_line":"            return check_num_instances_quota(context, instance_type, min_count,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_5d5ff440","line":747,"range":{"start_line":747,"start_character":18,"end_line":747,"end_character":63},"in_reply_to":"3f1d235d_35b87cc6","updated":"2017-07-12 22:42:16.000000000","message":"Yeah it\u0027s fine to remove here.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":759,"context_line":"                         ram\u003dreq_ram)"},{"line_number":760,"context_line":"        (overs, reqs, total_alloweds, useds) \u003d get_over_quota_detail("},{"line_number":761,"context_line":"            headroom, overs, quotas, requested)"},{"line_number":762,"context_line":"        params \u003d {\u0027overs\u0027: overs, \u0027pid\u0027: context.project_id,"},{"line_number":763,"context_line":"                  \u0027min_count\u0027: min_count, \u0027max_count\u0027: max_count,"},{"line_number":764,"context_line":"                  \u0027msg\u0027: msg}"},{"line_number":765,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_7505dec1","line":762,"range":{"start_line":762,"start_character":41,"end_line":762,"end_character":59},"updated":"2017-07-11 20:55:09.000000000","message":"Shouldn\u0027t this be:\n\nproject_id if project_id else context.project_id\n\nSince it\u0027s over-ridable?","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":759,"context_line":"                         ram\u003dreq_ram)"},{"line_number":760,"context_line":"        (overs, reqs, total_alloweds, useds) \u003d get_over_quota_detail("},{"line_number":761,"context_line":"            headroom, overs, quotas, requested)"},{"line_number":762,"context_line":"        params \u003d {\u0027overs\u0027: overs, \u0027pid\u0027: context.project_id,"},{"line_number":763,"context_line":"                  \u0027min_count\u0027: min_count, \u0027max_count\u0027: max_count,"},{"line_number":764,"context_line":"                  \u0027msg\u0027: msg}"},{"line_number":765,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_15e0e0cb","line":762,"range":{"start_line":762,"start_character":41,"end_line":762,"end_character":59},"in_reply_to":"3f1d235d_7505dec1","updated":"2017-07-12 02:27:38.000000000","message":"Yes, thank you for catching that.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":763,"context_line":"                  \u0027min_count\u0027: min_count, \u0027max_count\u0027: max_count,"},{"line_number":764,"context_line":"                  \u0027msg\u0027: msg}"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":"        if min_count \u003d\u003d max_count:"},{"line_number":767,"context_line":"            LOG.debug(\"%(overs)s quota exceeded for %(pid)s,\""},{"line_number":768,"context_line":"                      \" tried to run %(min_count)d instances. \""},{"line_number":769,"context_line":"                      \"%(msg)s\", params)"},{"line_number":770,"context_line":"        else:"},{"line_number":771,"context_line":"            LOG.debug(\"%(overs)s quota exceeded for %(pid)s,\""},{"line_number":772,"context_line":"                      \" tried to run between %(min_count)d and\""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_e737054d","line":769,"range":{"start_line":766,"start_character":8,"end_line":769,"end_character":40},"updated":"2017-07-11 20:55:09.000000000","message":"If we\u0027re rechecking quotas and min_count and max_count are 0, this error message gets weird. It would say something like \"tried to run 0 instances\".","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":763,"context_line":"                  \u0027min_count\u0027: min_count, \u0027max_count\u0027: max_count,"},{"line_number":764,"context_line":"                  \u0027msg\u0027: msg}"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":"        if min_count \u003d\u003d max_count:"},{"line_number":767,"context_line":"            LOG.debug(\"%(overs)s quota exceeded for %(pid)s,\""},{"line_number":768,"context_line":"                      \" tried to run %(min_count)d instances. \""},{"line_number":769,"context_line":"                      \"%(msg)s\", params)"},{"line_number":770,"context_line":"        else:"},{"line_number":771,"context_line":"            LOG.debug(\"%(overs)s quota exceeded for %(pid)s,\""},{"line_number":772,"context_line":"                      \" tried to run between %(min_count)d and\""}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_b5d00cf9","line":769,"range":{"start_line":766,"start_character":8,"end_line":769,"end_character":40},"in_reply_to":"3f1d235d_e737054d","updated":"2017-07-12 02:27:38.000000000","message":"Hm, good point. I\u0027ll add a check for 0 to bypass this message in that case.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2feeaa70ab1001b89db8a13a2f195791b2a1d33","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        quotas \u003d exc.kwargs[\u0027quotas\u0027]"},{"line_number":727,"context_line":"        overs \u003d exc.kwargs[\u0027overs\u0027]"},{"line_number":728,"context_line":"        usages \u003d exc.kwargs[\u0027usages\u0027]"},{"line_number":729,"context_line":"        if min_count \u003d\u003d max_count \u003d\u003d 0:"},{"line_number":730,"context_line":"            req_cores \u003d orig_num_req * instance_type.vcpus"},{"line_number":731,"context_line":"            req_ram \u003d orig_num_req * instance_type.memory_mb"},{"line_number":732,"context_line":"            requested \u003d {\u0027instances\u0027: orig_num_req, \u0027cores\u0027: req_cores,"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_c6a754bd","line":729,"range":{"start_line":729,"start_character":33,"end_line":729,"end_character":38},"updated":"2017-07-16 14:05:03.000000000","message":"I would add a comment that this is for the recheck case.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"76fe8d6cec4d392dbaf7aac132fae82905e7e33c","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        quotas \u003d exc.kwargs[\u0027quotas\u0027]"},{"line_number":727,"context_line":"        overs \u003d exc.kwargs[\u0027overs\u0027]"},{"line_number":728,"context_line":"        usages \u003d exc.kwargs[\u0027usages\u0027]"},{"line_number":729,"context_line":"        if min_count \u003d\u003d max_count \u003d\u003d 0:"},{"line_number":730,"context_line":"            req_cores \u003d orig_num_req * instance_type.vcpus"},{"line_number":731,"context_line":"            req_ram \u003d orig_num_req * instance_type.memory_mb"},{"line_number":732,"context_line":"            requested \u003d {\u0027instances\u0027: orig_num_req, \u0027cores\u0027: req_cores,"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_07264acf","line":729,"range":{"start_line":729,"start_character":33,"end_line":729,"end_character":38},"in_reply_to":"1f1a1f67_c6a754bd","updated":"2017-07-17 19:36:56.000000000","message":"Can do.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2feeaa70ab1001b89db8a13a2f195791b2a1d33","unresolved":false,"context_lines":[{"line_number":728,"context_line":"        usages \u003d exc.kwargs[\u0027usages\u0027]"},{"line_number":729,"context_line":"        if min_count \u003d\u003d max_count \u003d\u003d 0:"},{"line_number":730,"context_line":"            req_cores \u003d orig_num_req * instance_type.vcpus"},{"line_number":731,"context_line":"            req_ram \u003d orig_num_req * instance_type.memory_mb"},{"line_number":732,"context_line":"            requested \u003d {\u0027instances\u0027: orig_num_req, \u0027cores\u0027: req_cores,"},{"line_number":733,"context_line":"                         \u0027ram\u0027: req_ram}"},{"line_number":734,"context_line":"            (overs, reqs, total_alloweds, useds) \u003d get_over_quota_detail("}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_a6a8d88b","line":731,"range":{"start_line":731,"start_character":22,"end_line":731,"end_character":34},"updated":"2017-07-16 14:05:03.000000000","message":"Might be good to add a comment that this is when rechecking quotas after creating multiple instances in the same request and we went over.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"76fe8d6cec4d392dbaf7aac132fae82905e7e33c","unresolved":false,"context_lines":[{"line_number":728,"context_line":"        usages \u003d exc.kwargs[\u0027usages\u0027]"},{"line_number":729,"context_line":"        if min_count \u003d\u003d max_count \u003d\u003d 0:"},{"line_number":730,"context_line":"            req_cores \u003d orig_num_req * instance_type.vcpus"},{"line_number":731,"context_line":"            req_ram \u003d orig_num_req * instance_type.memory_mb"},{"line_number":732,"context_line":"            requested \u003d {\u0027instances\u0027: orig_num_req, \u0027cores\u0027: req_cores,"},{"line_number":733,"context_line":"                         \u0027ram\u0027: req_ram}"},{"line_number":734,"context_line":"            (overs, reqs, total_alloweds, useds) \u003d get_over_quota_detail("}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_e7222eb9","line":731,"range":{"start_line":731,"start_character":22,"end_line":731,"end_character":34},"in_reply_to":"1f1a1f67_a6a8d88b","updated":"2017-07-17 19:36:56.000000000","message":"Can do.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":696,"context_line":"        if deltas.get(\u0027cores\u0027):"},{"line_number":697,"context_line":"            hc \u003d headroom.get(\u0027instances\u0027, 1) * deltas[\u0027cores\u0027]"},{"line_number":698,"context_line":"            headroom[\u0027cores\u0027] \u003d hc / deltas.get(\u0027instances\u0027, 1)"},{"line_number":699,"context_line":"        else:"},{"line_number":700,"context_line":"            headroom[\u0027cores\u0027] \u003d headroom.get(\u0027instances\u0027, 1)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    # If quota_ram is unlimited [-1]:"},{"line_number":703,"context_line":"    # - set ram headroom based on instances headroom:"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_ac074518","line":700,"range":{"start_line":699,"start_character":8,"end_line":700,"end_character":60},"updated":"2017-07-19 01:58:18.000000000","message":"This is latent but we don\u0027t have a test for this case. Not a problem for your change.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":705,"context_line":"        if deltas.get(\u0027ram\u0027):"},{"line_number":706,"context_line":"            hr \u003d headroom.get(\u0027instances\u0027, 1) * deltas[\u0027ram\u0027]"},{"line_number":707,"context_line":"            headroom[\u0027ram\u0027] \u003d hr / deltas.get(\u0027instances\u0027, 1)"},{"line_number":708,"context_line":"        else:"},{"line_number":709,"context_line":"            headroom[\u0027ram\u0027] \u003d headroom.get(\u0027instances\u0027, 1)"},{"line_number":710,"context_line":""},{"line_number":711,"context_line":"    return headroom"},{"line_number":712,"context_line":""}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_6c15ad3f","line":709,"range":{"start_line":708,"start_character":8,"end_line":709,"end_character":58},"updated":"2017-07-19 01:58:18.000000000","message":"same","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"}],"nova/conductor/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"447945d336b40888f4f8c17ad5b1a58ab15d5b6a","unresolved":false,"context_lines":[{"line_number":972,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":973,"context_line":"                        except exception.TooManyInstances:"},{"line_number":974,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":975,"context_line":"                                instance.destroy()"},{"line_number":976,"context_line":""},{"line_number":977,"context_line":"            # send a state update notification for the initial create to"},{"line_number":978,"context_line":"            # show it going from non-existent to BUILDING"}],"source_content_type":"text/x-python","patch_set":57,"id":"3f1d235d_40209848","line":975,"updated":"2017-07-05 16:42:54.000000000","message":"I wonder if this is going to be confusing to people. Before, if they got their instance back, they were good on quota. Now, they might get an instance, a couple seconds pass as we queue up for conductor and then .. poof, it\u0027s gone. Seems like we should set this to ERROR and log an action that says \"please deposit another quarter to continue this call...\"","commit_id":"7f119036233365dfc33da9ca27de505f72b97622"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b8594956ef84a77d95d9a668be4cf3451a8b6c80","unresolved":false,"context_lines":[{"line_number":972,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":973,"context_line":"                        except exception.TooManyInstances:"},{"line_number":974,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":975,"context_line":"                                instance.destroy()"},{"line_number":976,"context_line":""},{"line_number":977,"context_line":"            # send a state update notification for the initial create to"},{"line_number":978,"context_line":"            # show it going from non-existent to BUILDING"}],"source_content_type":"text/x-python","patch_set":57,"id":"3f1d235d_c39b9215","line":975,"in_reply_to":"3f1d235d_40209848","updated":"2017-07-05 17:31:38.000000000","message":"Yeah, that\u0027s a good point. I wasn\u0027t thinking of how they get an instance back from the beginning (from the REST API call) even though it\u0027s sort of a placeholder (made from BuildRequest). So it would be not cool to have it disappear.\n\nFor the sake of brainstorming, I was thinking about how we could \"do the right thing\" and not give back an instance if a race went over quota, and to do it we\u0027d have to count BuildRequests as part of counting instances. Maybe we could actually do that without much trouble ... hmmm .... Because really, since BuildRequests count as an instance, we should be counting them anyway.","commit_id":"7f119036233365dfc33da9ca27de505f72b97622"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffb00ed76670aebed352c173122979729cdaa2fa","unresolved":false,"context_lines":[{"line_number":972,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":973,"context_line":"                        except exception.TooManyInstances:"},{"line_number":974,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":975,"context_line":"                                instance.destroy()"},{"line_number":976,"context_line":""},{"line_number":977,"context_line":"            # send a state update notification for the initial create to"},{"line_number":978,"context_line":"            # show it going from non-existent to BUILDING"}],"source_content_type":"text/x-python","patch_set":57,"id":"3f1d235d_09c76b66","line":975,"in_reply_to":"3f1d235d_c39b9215","updated":"2017-07-05 18:11:17.000000000","message":"Just closing the loop here:\n\nAs we discussed on IRC, we cant\u0027 do this because the sum of BuildRequest and Instance objects is not atomically correct all the time since they come from separate databases.","commit_id":"7f119036233365dfc33da9ca27de505f72b97622"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":979,"context_line":"                        try:"},{"line_number":980,"context_line":"                            compute_utils.check_num_instances_quota("},{"line_number":981,"context_line":"                                context, instance.flavor, 0, 0,"},{"line_number":982,"context_line":"                                project_id\u003dcontext.project_id,"},{"line_number":983,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":984,"context_line":"                        except exception.TooManyInstances as exc:"},{"line_number":985,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":986,"context_line":"                                updates \u003d {\u0027vm_state\u0027: vm_states.ERROR,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_da1f416e","line":983,"range":{"start_line":982,"start_character":32,"end_line":983,"end_character":55},"updated":"2017-07-11 20:55:09.000000000","message":"Same comment as in the API code, we don\u0027t really need to provide these kwargs.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":979,"context_line":"                        try:"},{"line_number":980,"context_line":"                            compute_utils.check_num_instances_quota("},{"line_number":981,"context_line":"                                context, instance.flavor, 0, 0,"},{"line_number":982,"context_line":"                                project_id\u003dcontext.project_id,"},{"line_number":983,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":984,"context_line":"                        except exception.TooManyInstances as exc:"},{"line_number":985,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":986,"context_line":"                                updates \u003d {\u0027vm_state\u0027: vm_states.ERROR,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_95cbd048","line":983,"range":{"start_line":982,"start_character":32,"end_line":983,"end_character":55},"in_reply_to":"3f1d235d_da1f416e","updated":"2017-07-12 02:27:38.000000000","message":"Ack.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5606207ded920bfb422b28c78a850fa88a7747ed","unresolved":false,"context_lines":[{"line_number":981,"context_line":"                                context, instance.flavor, 0, 0,"},{"line_number":982,"context_line":"                                project_id\u003dcontext.project_id,"},{"line_number":983,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":984,"context_line":"                        except exception.TooManyInstances as exc:"},{"line_number":985,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":986,"context_line":"                                updates \u003d {\u0027vm_state\u0027: vm_states.ERROR,"},{"line_number":987,"context_line":"                                           \u0027task_state\u0027: None}"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_9808fa28","line":984,"range":{"start_line":984,"start_character":41,"end_line":984,"end_character":57},"updated":"2017-07-12 22:42:16.000000000","message":"I think we have the same issue as mentioned in the API for cells v1:\n\nhttps://review.openstack.org/#/c/416521/58/nova/compute/api.py@1163\n\nWhich is, essentially, if we\u0027re doing multi-create and we go over quota, I think we have to do something about the other instances we created in this same request. Maybe it depends on what the original min_count was - if requested min 3 and max is 9, and we created 6 before going over quota, do we delete 3 to get back down to the requested min?","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f8b6c418f4211f2cea4f27c2f6511a47127a93a","unresolved":false,"context_lines":[{"line_number":981,"context_line":"                                context, instance.flavor, 0, 0,"},{"line_number":982,"context_line":"                                project_id\u003dcontext.project_id,"},{"line_number":983,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":984,"context_line":"                        except exception.TooManyInstances as exc:"},{"line_number":985,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":986,"context_line":"                                updates \u003d {\u0027vm_state\u0027: vm_states.ERROR,"},{"line_number":987,"context_line":"                                           \u0027task_state\u0027: None}"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_4631490b","line":984,"range":{"start_line":984,"start_character":41,"end_line":984,"end_character":57},"in_reply_to":"1f1a1f67_77abecd5","updated":"2017-07-13 15:22:42.000000000","message":"Yup.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5622a6923f922dd1281a449adb655860fd9a5e96","unresolved":false,"context_lines":[{"line_number":981,"context_line":"                                context, instance.flavor, 0, 0,"},{"line_number":982,"context_line":"                                project_id\u003dcontext.project_id,"},{"line_number":983,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":984,"context_line":"                        except exception.TooManyInstances as exc:"},{"line_number":985,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":986,"context_line":"                                updates \u003d {\u0027vm_state\u0027: vm_states.ERROR,"},{"line_number":987,"context_line":"                                           \u0027task_state\u0027: None}"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_f355d11e","line":984,"range":{"start_line":984,"start_character":41,"end_line":984,"end_character":57},"in_reply_to":"1f1a1f67_9808fa28","updated":"2017-07-12 23:48:44.000000000","message":"So here, we\u0027re not going to delete any instances, we\u0027re going to set them to ERROR state. Dan pointed out in an earlier PS that by the time we\u0027re in conductor, we\u0027ve already told the user we\u0027re booting N instances for them and so they should see those resultant objects even if we fail out here. So we were thinking to set them to ERROR state with a fault message containing the TooManyInstances error message.\n\nWe also need to delete the InstanceMapping, BuildRequest, and RequestSpec.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3d6794617d153ddb2656c205234e105cce7f9ea9","unresolved":false,"context_lines":[{"line_number":981,"context_line":"                                context, instance.flavor, 0, 0,"},{"line_number":982,"context_line":"                                project_id\u003dcontext.project_id,"},{"line_number":983,"context_line":"                                user_id\u003dcontext.user_id)"},{"line_number":984,"context_line":"                        except exception.TooManyInstances as exc:"},{"line_number":985,"context_line":"                            with excutils.save_and_reraise_exception():"},{"line_number":986,"context_line":"                                updates \u003d {\u0027vm_state\u0027: vm_states.ERROR,"},{"line_number":987,"context_line":"                                           \u0027task_state\u0027: None}"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_77abecd5","line":984,"range":{"start_line":984,"start_character":41,"end_line":984,"end_character":57},"in_reply_to":"1f1a1f67_f355d11e","updated":"2017-07-13 00:00:05.000000000","message":"Good point, by this time we\u0027ve done the RPC cast to conductor and returned a 202 to the API user.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2feeaa70ab1001b89db8a13a2f195791b2a1d33","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"            if instance is None:"},{"line_number":1056,"context_line":"                continue"},{"line_number":1057,"context_line":"            updates \u003d {\u0027vm_state\u0027: vm_states.ERROR, \u0027task_state\u0027: None}"},{"line_number":1058,"context_line":"            filter_props \u003d request_spec.to_legacy_filter_properties_dict()"},{"line_number":1059,"context_line":"            self._set_vm_state_and_notify(context, instance.uuid,"},{"line_number":1060,"context_line":"                                          \u0027build_instances\u0027, updates, exc,"},{"line_number":1061,"context_line":"                                          filter_props)"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_c6be34b3","line":1058,"range":{"start_line":1058,"start_character":12,"end_line":1058,"end_character":74},"updated":"2017-07-16 14:05:03.000000000","message":"I think this might be the wrong thing, I\u0027m seeing other places passing this thing in:\n\nlegacy_spec \u003d request_spec.to_legacy_request_spec_dict()","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d7003999925fd8094bebd757ef16b0cbbed8e70d","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"            if instance is None:"},{"line_number":1056,"context_line":"                continue"},{"line_number":1057,"context_line":"            updates \u003d {\u0027vm_state\u0027: vm_states.ERROR, \u0027task_state\u0027: None}"},{"line_number":1058,"context_line":"            filter_props \u003d request_spec.to_legacy_filter_properties_dict()"},{"line_number":1059,"context_line":"            self._set_vm_state_and_notify(context, instance.uuid,"},{"line_number":1060,"context_line":"                                          \u0027build_instances\u0027, updates, exc,"},{"line_number":1061,"context_line":"                                          filter_props)"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_498a0c6a","line":1058,"range":{"start_line":1058,"start_character":12,"end_line":1058,"end_character":74},"in_reply_to":"1f1a1f67_c6be34b3","updated":"2017-07-18 02:45:42.000000000","message":"Gah, you\u0027re right. Good eye, thanks for catching that.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":977,"context_line":"        # than their allowed quota in the event of a race. This is"},{"line_number":978,"context_line":"        # configurable because it can be expensive if strict quota"},{"line_number":979,"context_line":"        # limits are not required in a deployment."},{"line_number":980,"context_line":"        if CONF.quota.recheck_quota:"},{"line_number":981,"context_line":"            try:"},{"line_number":982,"context_line":"                compute_utils.check_num_instances_quota("},{"line_number":983,"context_line":"                    context, instance.flavor, 0, 0,"},{"line_number":984,"context_line":"                    orig_num_req\u003dlen(build_requests))"},{"line_number":985,"context_line":"            except exception.TooManyInstances as exc:"},{"line_number":986,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":987,"context_line":"                    self._cleanup_build_artifacts(context, exc, instances,"},{"line_number":988,"context_line":"                                                  build_requests,"},{"line_number":989,"context_line":"                                                  request_specs)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"        for (build_request, request_spec, host, instance) in six.moves.zip("},{"line_number":992,"context_line":"                build_requests, request_specs, hosts, instances):"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_ec4f7d68","line":989,"range":{"start_line":980,"start_character":8,"end_line":989,"end_character":64},"updated":"2017-07-19 01:58:18.000000000","message":"The coverage report says we\u0027re missing a unit test for this, but it looks like https://review.openstack.org/#/c/416521/63/nova/tests/unit/conductor/test_conductor.py@1689 should be testing this - what am I missing?","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":1052,"context_line":"                instances, build_requests, request_specs):"},{"line_number":1053,"context_line":"            # Skip placeholders that were buried in cell0 or had their"},{"line_number":1054,"context_line":"            # build requests deleted by the user before instance create."},{"line_number":1055,"context_line":"            if instance is None:"},{"line_number":1056,"context_line":"                continue"},{"line_number":1057,"context_line":"            updates \u003d {\u0027vm_state\u0027: vm_states.ERROR, \u0027task_state\u0027: None}"},{"line_number":1058,"context_line":"            legacy_spec \u003d request_spec.to_legacy_request_spec_dict()"},{"line_number":1059,"context_line":"            self._set_vm_state_and_notify(context, instance.uuid,"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_4c3e71b8","line":1056,"range":{"start_line":1055,"start_character":12,"end_line":1056,"end_character":24},"updated":"2017-07-19 01:58:18.000000000","message":"We don\u0027t have a unit test covering this case.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":1062,"context_line":"            # Be paranoid about artifacts being deleted underneath us."},{"line_number":1063,"context_line":"            try:"},{"line_number":1064,"context_line":"                build_request.destroy()"},{"line_number":1065,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":1066,"context_line":"                pass"},{"line_number":1067,"context_line":"            try:"},{"line_number":1068,"context_line":"                request_spec.destroy()"},{"line_number":1069,"context_line":"            except exception.RequestSpecNotFound:"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_0c44f945","line":1066,"range":{"start_line":1065,"start_character":12,"end_line":1066,"end_character":20},"updated":"2017-07-19 01:58:18.000000000","message":"No test coverage for this or the NotFound below. These could be handled in a follow up.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"}],"nova/conf/quota.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"447945d336b40888f4f8c17ad5b1a58ab15d5b6a","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"* A positive integer or 0."},{"line_number":349,"context_line":"* -1 to disable the quota."},{"line_number":350,"context_line":"\"\"\"),"},{"line_number":351,"context_line":"]"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":57,"id":"3f1d235d_c0c26859","line":350,"updated":"2017-07-05 16:42:54.000000000","message":"This might should be a separate patch?","commit_id":"7f119036233365dfc33da9ca27de505f72b97622"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b8594956ef84a77d95d9a668be4cf3451a8b6c80","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"* A positive integer or 0."},{"line_number":349,"context_line":"* -1 to disable the quota."},{"line_number":350,"context_line":"\"\"\"),"},{"line_number":351,"context_line":"]"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":57,"id":"3f1d235d_43542253","line":350,"in_reply_to":"3f1d235d_c0c26859","updated":"2017-07-05 17:31:38.000000000","message":"It could be, yeah. I think I lumped it in with this one to start, since it\u0027s meaningless without \"counting instances\" but it could be a separate patch that goes after this one, leaving a small window where resize downs could potentially create an \"OverQuota upon revert\" situation. Or, the \"pending resizes\" patch could do before \"counting instances\"?","commit_id":"7f119036233365dfc33da9ca27de505f72b97622"}],"nova/exception.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1420,"context_line":"class PendingResizesExceeded(QuotaError):"},{"line_number":1421,"context_line":"    msg_fmt \u003d _(\"Maximum number of pending resizes exceeded\")"},{"line_number":1422,"context_line":""},{"line_number":1423,"context_line":""},{"line_number":1424,"context_line":"class AggregateError(NovaException):"},{"line_number":1425,"context_line":"    msg_fmt \u003d _(\"Aggregate %(aggregate_id)s: action \u0027%(action)s\u0027 \""},{"line_number":1426,"context_line":"                \"caused an error: %(reason)s.\")"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_55d538e4","line":1423,"updated":"2017-07-12 02:27:38.000000000","message":"-1 Oops, this should be in the next patch, not here. Missed it when I moved the other stuff related to it.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"}],"nova/objects/instance.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e7a8e85dedcb8c9c7e33ddc2e413eea7ddfc4675","unresolved":false,"context_lines":[{"line_number":1427,"context_line":""},{"line_number":1428,"context_line":"        project_result \u003d project_query.first()"},{"line_number":1429,"context_line":"        LOG.debug(\u0027project_result \u003d %s\u0027, project_result)"},{"line_number":1430,"context_line":"        counts \u003d {\u0027project\u0027: {\u0027instances\u0027: project_result[0] or 0,"},{"line_number":1431,"context_line":"                              \u0027cores\u0027: project_result[1] or 0,"},{"line_number":1432,"context_line":"                              \u0027ram\u0027: project_result[2] or 0}}"},{"line_number":1433,"context_line":"        LOG.debug(\u0027counts \u003d %s\u0027, counts)"}],"source_content_type":"text/x-python","patch_set":61,"id":"1f1a1f67_259cccbe","line":1430,"updated":"2017-07-15 01:13:02.000000000","message":"The results are coming back as Decimal objects, maybe because it\u0027s not a model query:\n\nhttp://logs.openstack.org/21/416521/61/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/1328370/logs/screen-n-api.txt.gz#_Jul_14_22_13_44_084898\n\ncounts \u003d {\u0027project\u0027: {\u0027instances\u0027: 2, \u0027ram\u0027: Decimal(\u0027128\u0027), \u0027cores\u0027: Decimal(\u00272\u0027)}}\n\nWe should just cast to int like this does, which also uses .sum():\n\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L853","commit_id":"fd7dba15bc14001a6ade79d55a978c766bb0f5d2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4570afd1e8d7385c03ee7c4886d381537f64b70b","unresolved":false,"context_lines":[{"line_number":1427,"context_line":""},{"line_number":1428,"context_line":"        project_result \u003d project_query.first()"},{"line_number":1429,"context_line":"        LOG.debug(\u0027project_result \u003d %s\u0027, project_result)"},{"line_number":1430,"context_line":"        counts \u003d {\u0027project\u0027: {\u0027instances\u0027: project_result[0] or 0,"},{"line_number":1431,"context_line":"                              \u0027cores\u0027: project_result[1] or 0,"},{"line_number":1432,"context_line":"                              \u0027ram\u0027: project_result[2] or 0}}"},{"line_number":1433,"context_line":"        LOG.debug(\u0027counts \u003d %s\u0027, counts)"}],"source_content_type":"text/x-python","patch_set":61,"id":"1f1a1f67_85c16083","line":1430,"in_reply_to":"1f1a1f67_259cccbe","updated":"2017-07-15 01:19:08.000000000","message":"A-ha, thanks. I figured it was something like that, but didn\u0027t know how it could be happening since I copied another working query. BUT like you said, that one is using the model_query helper, and probably that\u0027s the key.\n\nThanks for pointing out the other example, that one looks a lot better than what I did.","commit_id":"fd7dba15bc14001a6ade79d55a978c766bb0f5d2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"915f91cc64324588bacaf6c11ead417cd4bb5068","unresolved":false,"context_lines":[{"line_number":1408,"context_line":"                                                vm_state)"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"    @staticmethod"},{"line_number":1411,"context_line":"    @db.select_db_reader_mode"},{"line_number":1412,"context_line":"    def _get_counts_in_db(context, project_id, user_id\u003dNone):"},{"line_number":1413,"context_line":"        # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py:"},{"line_number":1414,"context_line":"        # It would be better to have vm_state not be nullable"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_24184573","line":1411,"range":{"start_line":1411,"start_character":4,"end_line":1411,"end_character":29},"updated":"2017-07-15 21:35:18.000000000","message":"I don\u0027t know if it matters, but you could change this back to:\n\n@db_api.pick_context_manager_reader\n\nSince _get_count_by_vm_state_in_db is using that for consistency. I\u0027m not entirely sure if either is better or makes a difference. I think select_db_reader_mode just makes a distinction if a reader slave should be used.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2943177080470caa73772c806f359937c8987af0","unresolved":false,"context_lines":[{"line_number":1408,"context_line":"                                                vm_state)"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"    @staticmethod"},{"line_number":1411,"context_line":"    @db.select_db_reader_mode"},{"line_number":1412,"context_line":"    def _get_counts_in_db(context, project_id, user_id\u003dNone):"},{"line_number":1413,"context_line":"        # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py:"},{"line_number":1414,"context_line":"        # It would be better to have vm_state not be nullable"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_26a25ffd","line":1411,"range":{"start_line":1411,"start_character":4,"end_line":1411,"end_character":29},"in_reply_to":"1f1a1f67_060be323","updated":"2017-07-18 02:10:05.000000000","message":"Nevermind about going through all - there are times we don\u0027t want to use a slave connection, like if we just wrote it and need to retrieve it. I do sort of wonder then if we want to rely on slaves to provide information about counts, when we\u0027re bursting and creating a bunch of instances, we don\u0027t want to get those counts wrong because we\u0027re getting stale data.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3c6d4a31bbc938da42834fc6efd29b0765c7ba60","unresolved":false,"context_lines":[{"line_number":1408,"context_line":"                                                vm_state)"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"    @staticmethod"},{"line_number":1411,"context_line":"    @db.select_db_reader_mode"},{"line_number":1412,"context_line":"    def _get_counts_in_db(context, project_id, user_id\u003dNone):"},{"line_number":1413,"context_line":"        # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py:"},{"line_number":1414,"context_line":"        # It would be better to have vm_state not be nullable"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_67317e5a","line":1411,"range":{"start_line":1411,"start_character":4,"end_line":1411,"end_character":29},"in_reply_to":"1f1a1f67_24184573","updated":"2017-07-17 19:25:03.000000000","message":"I think this should be select_db_reader_mode because then it would use the read slave DB if one is available. Really the other method should be using select_db_reader_mode too, as well as anything else that does a read. These made a significant difference in DB load at Yahoo (they use the read slave DB configuration there).","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"647a9f5bccc6c836c5add26a1870019a8fba6c42","unresolved":false,"context_lines":[{"line_number":1408,"context_line":"                                                vm_state)"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"    @staticmethod"},{"line_number":1411,"context_line":"    @db.select_db_reader_mode"},{"line_number":1412,"context_line":"    def _get_counts_in_db(context, project_id, user_id\u003dNone):"},{"line_number":1413,"context_line":"        # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py:"},{"line_number":1414,"context_line":"        # It would be better to have vm_state not be nullable"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_c93e1cb7","line":1411,"range":{"start_line":1411,"start_character":4,"end_line":1411,"end_character":29},"in_reply_to":"1f1a1f67_26a25ffd","updated":"2017-07-18 02:43:24.000000000","message":"Hm, that\u0027s a good point. I agree that counting for quota checks is something we don\u0027t want to potentially be reading stale data on. So I\u0027ll change this to @db_api.pick_context_manager_reader","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"28a6f91f75bc179b66b24e0d1d19c0a3b2e67038","unresolved":false,"context_lines":[{"line_number":1408,"context_line":"                                                vm_state)"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"    @staticmethod"},{"line_number":1411,"context_line":"    @db.select_db_reader_mode"},{"line_number":1412,"context_line":"    def _get_counts_in_db(context, project_id, user_id\u003dNone):"},{"line_number":1413,"context_line":"        # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py:"},{"line_number":1414,"context_line":"        # It would be better to have vm_state not be nullable"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_a87f3aa9","line":1411,"range":{"start_line":1411,"start_character":4,"end_line":1411,"end_character":29},"in_reply_to":"1f1a1f67_40bbe4c3","updated":"2017-07-18 20:20:10.000000000","message":"Cool, thanks for sharing the link","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ff1a85f7a31c37f5a897d63927134dc5a95c1c3","unresolved":false,"context_lines":[{"line_number":1408,"context_line":"                                                vm_state)"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"    @staticmethod"},{"line_number":1411,"context_line":"    @db.select_db_reader_mode"},{"line_number":1412,"context_line":"    def _get_counts_in_db(context, project_id, user_id\u003dNone):"},{"line_number":1413,"context_line":"        # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py:"},{"line_number":1414,"context_line":"        # It would be better to have vm_state not be nullable"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_060be323","line":1411,"range":{"start_line":1411,"start_character":4,"end_line":1411,"end_character":29},"in_reply_to":"1f1a1f67_67317e5a","updated":"2017-07-18 02:02:07.000000000","message":"OK, cool, that works for me. Maybe we should report a bug at some point about auditing and using select_db_reader_mode for the places where pick_context_manager_reader is used.\n\nI don\u0027t know how easy it would be to set this up to compare before/after going through and making the changes globally in Nova, I\u0027ve never done it before. Just thinking out loud - not talking about doing any of that here, but the bug might help us track the work.","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f2181a4ff67208cc60062ec81b11713e01da1cb","unresolved":false,"context_lines":[{"line_number":1408,"context_line":"                                                vm_state)"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"    @staticmethod"},{"line_number":1411,"context_line":"    @db.select_db_reader_mode"},{"line_number":1412,"context_line":"    def _get_counts_in_db(context, project_id, user_id\u003dNone):"},{"line_number":1413,"context_line":"        # NOTE(melwitt): Copied from nova/db/sqlalchemy/api.py:"},{"line_number":1414,"context_line":"        # It would be better to have vm_state not be nullable"}],"source_content_type":"text/x-python","patch_set":62,"id":"1f1a1f67_40bbe4c3","line":1411,"range":{"start_line":1411,"start_character":4,"end_line":1411,"end_character":29},"in_reply_to":"1f1a1f67_c93e1cb7","updated":"2017-07-18 17:29:51.000000000","message":"\\o/ I sounded smart for a second.\n\nFWIW I only thought about this after reading:\n\nhttps://wiki.openstack.org/wiki/Slave_usage#When_a_slave_is_useful.2C_when_it_is_not","commit_id":"778ffd0bed4f1a7e5813abb476aace87062e1576"}],"nova/objects/instance_group.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f6b9318cb806722324b2cc99d3cfd26e7bfc775e","unresolved":false,"context_lines":[{"line_number":285,"context_line":"    def _count_by_project_and_user_from_db(context, project_id, user_id):"},{"line_number":286,"context_line":"        return context.session.query(api_models.InstanceGroup.id).\\"},{"line_number":287,"context_line":"                filter_by(project_id\u003dproject_id).\\"},{"line_number":288,"context_line":"                filter_by(user_id\u003duser_id).\\"},{"line_number":289,"context_line":"                count()"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    def obj_load_attr(self, attrname):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_dc5f503f","line":288,"updated":"2017-01-20 18:22:45.000000000","message":"I think this needs to skip the user_id filter if user_id\u003dNone, so we can do a count for both users and projects.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"}],"nova/objects/keypair.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ddd84c2c9f0487bbb01aa3fc64f53dde312b6015","unresolved":false,"context_lines":[{"line_number":31,"context_line":"KEYPAIR_TYPE_SSH \u003d \u0027ssh\u0027"},{"line_number":32,"context_line":"KEYPAIR_TYPE_X509 \u003d \u0027x509\u0027"},{"line_number":33,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":34,"context_line":"CONF \u003d conf.CONF"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"@db_api.api_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_4c9d8c58","line":34,"updated":"2017-03-15 23:39:47.000000000","message":"Is this necessary?","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"03c859fb6da177aa0a35bd4ac1700a25e23d9500","unresolved":false,"context_lines":[{"line_number":31,"context_line":"KEYPAIR_TYPE_SSH \u003d \u0027ssh\u0027"},{"line_number":32,"context_line":"KEYPAIR_TYPE_X509 \u003d \u0027x509\u0027"},{"line_number":33,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":34,"context_line":"CONF \u003d conf.CONF"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"@db_api.api_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_87de7d6b","line":34,"in_reply_to":"3a1ff146_4c9d8c58","updated":"2017-03-16 00:04:38.000000000","message":"Guh, residue from PS16. Will remove.","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"}],"nova/objects/quotas.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"04920231040e5495bbeca3b3161e85c5f8bbaece","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    #              get_all_by_project_and_user(), destroy_all_by_project(),"},{"line_number":63,"context_line":"    #              destroy_all_by_project_and_user(), get_class(),"},{"line_number":64,"context_line":"    #              get_default_class(), get_all_class_by_name(),"},{"line_number":65,"context_line":"    #              create_class(), update_class(), check_deltas()"},{"line_number":66,"context_line":"    VERSION \u003d \u00271.3\u0027"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_27edc925","line":65,"updated":"2017-03-15 23:51:07.000000000","message":"You can\u0027t do this, you need to bump the version.","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"03c859fb6da177aa0a35bd4ac1700a25e23d9500","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    #              get_all_by_project_and_user(), destroy_all_by_project(),"},{"line_number":63,"context_line":"    #              destroy_all_by_project_and_user(), get_class(),"},{"line_number":64,"context_line":"    #              get_default_class(), get_all_class_by_name(),"},{"line_number":65,"context_line":"    #              create_class(), update_class(), check_deltas()"},{"line_number":66,"context_line":"    VERSION \u003d \u00271.3\u0027"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_a747f91b","line":65,"in_reply_to":"3a1ff146_27edc925","updated":"2017-03-16 00:04:38.000000000","message":"Oops","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"04920231040e5495bbeca3b3161e85c5f8bbaece","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        self.obj_reset_changes()"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    @base.remotable_classmethod"},{"line_number":305,"context_line":"    def limit_check(cls, context, proj_values\u003dNone, project_id\u003dNone,"},{"line_number":306,"context_line":"                    user_id\u003dNone, **values):"},{"line_number":307,"context_line":"        \"\"\"Check quota limits.\"\"\""},{"line_number":308,"context_line":"        return quota.QUOTAS.limit_check("}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_67f7c1f5","line":305,"updated":"2017-03-15 23:51:07.000000000","message":"You can\u0027t change the method signature in an incompatible way regardless of what you do to the version.","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"03c859fb6da177aa0a35bd4ac1700a25e23d9500","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        self.obj_reset_changes()"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    @base.remotable_classmethod"},{"line_number":305,"context_line":"    def limit_check(cls, context, proj_values\u003dNone, project_id\u003dNone,"},{"line_number":306,"context_line":"                    user_id\u003dNone, **values):"},{"line_number":307,"context_line":"        \"\"\"Check quota limits.\"\"\""},{"line_number":308,"context_line":"        return quota.QUOTAS.limit_check("}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_27ba29c1","line":305,"in_reply_to":"3a1ff146_67f7c1f5","updated":"2017-03-16 00:04:38.000000000","message":"By that do you mean the ordering of the kwargs? I can put proj_values at the end, but I was thinking it didn\u0027t matter. I guess it would matter to callers that are not specifying by name and relying on order?","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6eab3055ea24d264948d8a0be24c0365dc0c91b1","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        self.obj_reset_changes()"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    @base.remotable_classmethod"},{"line_number":305,"context_line":"    def limit_check(cls, context, proj_values\u003dNone, project_id\u003dNone,"},{"line_number":306,"context_line":"                    user_id\u003dNone, **values):"},{"line_number":307,"context_line":"        \"\"\"Check quota limits.\"\"\""},{"line_number":308,"context_line":"        return quota.QUOTAS.limit_check("}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_27e149d1","line":305,"in_reply_to":"3a1ff146_67f7c1f5","updated":"2017-03-16 00:04:48.000000000","message":"Well, I guess this isn\u0027t technically incompatible because it is a kwarg among kwargs, but changing the order here seems evil. Also, since this takes **kwargs (which is terrible for an RPC method), it would probably be better to just split this out into a new method, especially since it\u0027s small. That also makes deprecation of the old behavior easier because it\u0027s a whole deprecated method instead of just a piece of one.","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"}],"nova/objects/security_group.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12ecc28fb2f3c414f8c18e1d22210da4197beb88","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        return self.uuid if \u0027uuid\u0027 in self else self.name"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    @base.remotable_classmethod"},{"line_number":112,"context_line":"    def get_counts(cls, context, project_id, user_id\u003dNone):"},{"line_number":113,"context_line":"        return cls._get_counts_from_db(context, project_id, user_id\u003duser_id)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_9e56c5d0","line":112,"updated":"2017-01-26 17:35:45.000000000","message":"Nit: Shouldn\u0027t that be on the SecurityGroupList? I am not sure.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7313476099e6fdba9bfcc517652eb1e32105e8cd","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        return self.uuid if \u0027uuid\u0027 in self else self.name"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    @base.remotable_classmethod"},{"line_number":112,"context_line":"    def get_counts(cls, context, project_id, user_id\u003dNone):"},{"line_number":113,"context_line":"        return cls._get_counts_from_db(context, project_id, user_id\u003duser_id)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_ebd0ea55","line":112,"in_reply_to":"3a461143_9e56c5d0","updated":"2017-01-26 17:55:26.000000000","message":"That might make more sense, yeah.","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ddd84c2c9f0487bbb01aa3fc64f53dde312b6015","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                    {\u0027project\u0027: {\u0027security_groups\u0027: \u003ccount across project\u003e},"},{"line_number":156,"context_line":"                     \u0027user\u0027: {\u0027security_groups\u0027: \u003ccount across user\u003e}}"},{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        return cls._get_counts_from_db(context, project_id, user_id\u003duser_id)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def make_secgroup_list(security_groups):"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_8c222400","line":158,"updated":"2017-03-15 23:39:47.000000000","message":"Where is the test for this?","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"03c859fb6da177aa0a35bd4ac1700a25e23d9500","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                    {\u0027project\u0027: {\u0027security_groups\u0027: \u003ccount across project\u003e},"},{"line_number":156,"context_line":"                     \u0027user\u0027: {\u0027security_groups\u0027: \u003ccount across user\u003e}}"},{"line_number":157,"context_line":"        \"\"\""},{"line_number":158,"context_line":"        return cls._get_counts_from_db(context, project_id, user_id\u003duser_id)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def make_secgroup_list(security_groups):"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_c7c5d53c","line":158,"in_reply_to":"3a1ff146_8c222400","updated":"2017-03-16 00:04:38.000000000","message":"Looks like I missed it. Will add.","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"}],"nova/quota.py":[{"author":{"_account_id":23882,"name":"Dan Peschman","email":"dpeschman@godaddy.com","username":"dtp"},"change_message_id":"3b387874afd21900933b2cbb0be4f2e737ac980a","unresolved":false,"context_lines":[{"line_number":1538,"context_line":"        total_count[\u0027instances\u0027] +\u003d 1"},{"line_number":1539,"context_line":"        total_count[\u0027cores\u0027] +\u003d instance.flavor.vcpus"},{"line_number":1540,"context_line":"        total_count[\u0027ram\u0027] +\u003d instance.flavor.memory_mb"},{"line_number":1541,"context_line":"    return total_count"},{"line_number":1542,"context_line":""},{"line_number":1543,"context_line":""},{"line_number":1544,"context_line":"def _security_group_count(context, project_id, user_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_1bbccb04","line":1541,"updated":"2017-01-13 14:04:02.000000000","message":"This sums usage from each of the cells, right?","commit_id":"f1ce97dcc63e078b8e4435cb98602eef4e25c979"},{"author":{"_account_id":23882,"name":"Dan Peschman","email":"dpeschman@godaddy.com","username":"dtp"},"change_message_id":"44820b12102553860c19cdf74b99d9637939f3f8","unresolved":false,"context_lines":[{"line_number":1538,"context_line":"        total_count[\u0027instances\u0027] +\u003d 1"},{"line_number":1539,"context_line":"        total_count[\u0027cores\u0027] +\u003d instance.flavor.vcpus"},{"line_number":1540,"context_line":"        total_count[\u0027ram\u0027] +\u003d instance.flavor.memory_mb"},{"line_number":1541,"context_line":"    return total_count"},{"line_number":1542,"context_line":""},{"line_number":1543,"context_line":""},{"line_number":1544,"context_line":"def _security_group_count(context, project_id, user_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_e73e7cb7","line":1541,"in_reply_to":"7a3c09a3_1bbccb04","updated":"2017-01-13 15:03:35.000000000","message":"What happens if a cell is unable to report its usage?","commit_id":"f1ce97dcc63e078b8e4435cb98602eef4e25c979"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f6b9318cb806722324b2cc99d3cfd26e7bfc775e","unresolved":false,"context_lines":[{"line_number":430,"context_line":"            if k in merged_quotas:"},{"line_number":431,"context_line":"                merged_quotas[k] \u003d min(merged_quotas[k], v)"},{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                merged_quotas[k] \u003d v"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        # Check the quotas and construct a list of the resources that"},{"line_number":436,"context_line":"        # would be put over limit by the desired values"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_d792ca9e","line":433,"updated":"2017-01-20 18:22:45.000000000","message":"Is that allowed? Are we not checking separate things against those?","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ee57d12daf1839fe215d8b2f9fb47207869ee295","unresolved":false,"context_lines":[{"line_number":430,"context_line":"            if k in merged_quotas:"},{"line_number":431,"context_line":"                merged_quotas[k] \u003d min(merged_quotas[k], v)"},{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                merged_quotas[k] \u003d v"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        # Check the quotas and construct a list of the resources that"},{"line_number":436,"context_line":"        # would be put over limit by the desired values"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_39d0261f","line":433,"in_reply_to":"5a3905b3_0af6aef8","updated":"2017-01-23 11:08:05.000000000","message":"Yeah, we are getting away with where we were sloppy before, because it happens they were only used in the special cases where there is no per user / per project difference. Its crazy in there.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"72c6d62799a486d1fd1d21f63e3d720462946e75","unresolved":false,"context_lines":[{"line_number":430,"context_line":"            if k in merged_quotas:"},{"line_number":431,"context_line":"                merged_quotas[k] \u003d min(merged_quotas[k], v)"},{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                merged_quotas[k] \u003d v"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        # Check the quotas and construct a list of the resources that"},{"line_number":436,"context_line":"        # would be put over limit by the desired values"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_0af6aef8","line":433,"in_reply_to":"5a3905b3_d791ea53","updated":"2017-01-23 09:40:13.000000000","message":"After examining the old system and thinking about it more, I think I understand your point. In the old code, limit_check was only used for per user counts of: keypairs, server group members, and security group rules. Contrasting with the quota_reserve code that checked a project-wide count and a per user count separately.\n\nI think I have to change this to consider both a count across a project and a count across a user and check against the project limit and user limit separately. And I think the keypairs, server group members, and security group rules are special cases where there isn\u0027t a project-wide quota for them and they are per user only.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a735889284c11dde55eb62edfb5b6f4c09dafd9a","unresolved":false,"context_lines":[{"line_number":430,"context_line":"            if k in merged_quotas:"},{"line_number":431,"context_line":"                merged_quotas[k] \u003d min(merged_quotas[k], v)"},{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                merged_quotas[k] \u003d v"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        # Check the quotas and construct a list of the resources that"},{"line_number":436,"context_line":"        # would be put over limit by the desired values"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_d791ea53","line":433,"in_reply_to":"5a3905b3_d792ca9e","updated":"2017-01-20 18:35:54.000000000","message":"So, if you look at the old code, it was already doing this, but with a bug in what it reported as the limit value in the exception. L434-436 in the old code checks against both project-wide quota and user quota, and raises if either of them were exceeded. Then it generates an exception with that data, and I found that the value it generated for the limit value it exceeded was not necessarily the one that went over (between project and user). Since nothing was tracking whether it was the project limit or the user limit that was surpassed. I hope that makes sense.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f82d60ea0cc87eb137a5ea81487d3167b66605a","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"def _instances_cores_ram_count(context, project_id, user_id):"},{"line_number":1523,"context_line":"    cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":1524,"context_line":"    greenthreads \u003d []"},{"line_number":1525,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id, \u0027user_id\u0027: user_id}"},{"line_number":1526,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":1527,"context_line":"        with nova_context.target_cell(context, cell_mapping):"},{"line_number":1528,"context_line":"            # Need to make a copy of the context with db_connection set before"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_979da25b","line":1525,"updated":"2017-01-20 18:31:12.000000000","message":"I was thinking about how the old system used to deal with user_id\u003dNone here:\n\nhttps://github.com/openstack/nova/blob/065cd6a8d69c1ec862e5b402a3150131f35b2420/nova/db/sqlalchemy/api.py#L1830","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"72c6d62799a486d1fd1d21f63e3d720462946e75","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"def _instances_cores_ram_count(context, project_id, user_id):"},{"line_number":1523,"context_line":"    cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":1524,"context_line":"    greenthreads \u003d []"},{"line_number":1525,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id, \u0027user_id\u0027: user_id}"},{"line_number":1526,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":1527,"context_line":"        with nova_context.target_cell(context, cell_mapping):"},{"line_number":1528,"context_line":"            # Need to make a copy of the context with db_connection set before"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_ea3ec289","line":1525,"in_reply_to":"5a3905b3_3a2773b7","updated":"2017-01-23 09:40:13.000000000","message":"Please disregard my comment here -- I explain why in my latest reply on L433.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3fd308f8207dd0f7b331a1a570303631e2c79d8b","unresolved":false,"context_lines":[{"line_number":1522,"context_line":"def _instances_cores_ram_count(context, project_id, user_id):"},{"line_number":1523,"context_line":"    cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":1524,"context_line":"    greenthreads \u003d []"},{"line_number":1525,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id, \u0027user_id\u0027: user_id}"},{"line_number":1526,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":1527,"context_line":"        with nova_context.target_cell(context, cell_mapping):"},{"line_number":1528,"context_line":"            # Need to make a copy of the context with db_connection set before"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_3a2773b7","line":1525,"in_reply_to":"5a3905b3_979da25b","updated":"2017-01-20 18:44:22.000000000","message":"Thanks for pointing that out. It looks like \u0027instances\u0027 has a bit different handling than the rest that are always counting per project or per project\u0026user. The code gives the choice to count per project or per project\u0026user for instances, for the usage sync.\n\nTracing the code, it looks like which path is taken hinges on whether or not user_id is passed during the quota usage refresh. One is when \u0027reserve\u0027 is called and the other is when \u0027quota_usage_refresh\u0027 is called via nova-manage.\n\nI think the old system always passed user_id when doing a \u0027reserve\u0027 for an instance, therefore the sync always filtered by user_id. I think at the heart of it, it should work the same as I explained earlier for server groups, count them by user and then the limit check will compare against the user limit if it exists, else it falls back to the project-wide default limit.\n\nLet me know if I\u0027ve missed your point there.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6953952dae944176f3e409ec4e73b20b731d947","unresolved":false,"context_lines":[{"line_number":1545,"context_line":""},{"line_number":1546,"context_line":""},{"line_number":1547,"context_line":"def _security_group_count(context, project_id, user_id):"},{"line_number":1548,"context_line":"    # TODO(melwitt): Should we assume a single cell here instead?"},{"line_number":1549,"context_line":"    cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":1550,"context_line":"    greenthreads \u003d []"},{"line_number":1551,"context_line":"    for cell_mapping in cell_mappings:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_41a7f94e","line":1548,"updated":"2017-01-20 17:56:28.000000000","message":"I think thats fine now we deprecated nova-network for everyone except cells v1, at least I think we have.","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f6b9318cb806722324b2cc99d3cfd26e7bfc775e","unresolved":false,"context_lines":[{"line_number":1572,"context_line":"    return objects.FixedIP.count_by_project(context, project_id)"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":""},{"line_number":1575,"context_line":"def _server_group_count(context, project_id, user_id):"},{"line_number":1576,"context_line":"    return objects.InstanceGroup.count_by_project_and_user(context, project_id,"},{"line_number":1577,"context_line":"                                                           user_id)"},{"line_number":1578,"context_line":""},{"line_number":1579,"context_line":""},{"line_number":1580,"context_line":"def _server_group_count_members_by_user(context, group, user_id):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_5727bbb5","line":1577,"range":{"start_line":1575,"start_character":0,"end_line":1577,"end_character":67},"updated":"2017-01-20 18:22:45.000000000","message":"Feels like this should allow user_id to be None, so we can do both types of counts here?","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a735889284c11dde55eb62edfb5b6f4c09dafd9a","unresolved":false,"context_lines":[{"line_number":1572,"context_line":"    return objects.FixedIP.count_by_project(context, project_id)"},{"line_number":1573,"context_line":""},{"line_number":1574,"context_line":""},{"line_number":1575,"context_line":"def _server_group_count(context, project_id, user_id):"},{"line_number":1576,"context_line":"    return objects.InstanceGroup.count_by_project_and_user(context, project_id,"},{"line_number":1577,"context_line":"                                                           user_id)"},{"line_number":1578,"context_line":""},{"line_number":1579,"context_line":""},{"line_number":1580,"context_line":"def _server_group_count_members_by_user(context, group, user_id):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a3905b3_57641a48","line":1577,"range":{"start_line":1575,"start_character":0,"end_line":1577,"end_character":67},"in_reply_to":"5a3905b3_5727bbb5","updated":"2017-01-20 18:35:54.000000000","message":"IIUC, server groups always have a project_id and user_id associated with them. That is, when they\u0027re created in the API, they have both set [1]. So the project vs user will only come into play when you\u0027re checking against the quota limits. If a per user limit has been created for a user, they get check against that, otherwise it falls back to the default per project limit.\n\nSo to me it seems we always count how many server groups belong to a user and check the per user limit if it exists, else fall back to the default (and defaults are always project-wide).\n\n[1] https://github.com/openstack/nova/blob/4a09c22/nova/api/openstack/compute/server_groups.py#L149-L155","commit_id":"dda1294da04ade4fc5a83104435967b54291ec91"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"eaeae27a9f5327ed49387050778443d6482f5a87","unresolved":false,"context_lines":[{"line_number":1605,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1606,"context_line":"    for instance in instances:"},{"line_number":1607,"context_line":"        instance_count \u003d 1"},{"line_number":1608,"context_line":"        cores_count \u003d instance.flavor.vcpus"},{"line_number":1609,"context_line":"        ram_count \u003d instance.flavor.memory_mb"},{"line_number":1610,"context_line":"        total_counts[\u0027project\u0027][\u0027instances\u0027] +\u003d instance_count"},{"line_number":1611,"context_line":"        total_counts[\u0027project\u0027][\u0027cores\u0027] +\u003d cores_count"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a461143_8b504660","line":1608,"updated":"2017-01-26 18:06:43.000000000","message":"I think we need to look here for resize pending instances and count the new flavor (and we should make sure that is set just after we check the qutoa)","commit_id":"4acd8c1c7d13c0a7896bb69e1acc0c6f05992c88"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"092451eb7bff9f99c39ea8425176219d11d55643","unresolved":false,"context_lines":[{"line_number":1610,"context_line":"            context_copy \u003d copy.copy(context)"},{"line_number":1611,"context_line":"            greenthreads.append(utils.spawn("},{"line_number":1612,"context_line":"                objects.InstanceList.get_by_filters,"},{"line_number":1613,"context_line":"                context_copy, filters, expected_attrs\u003d[\u0027flavor\u0027]))"},{"line_number":1614,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1615,"context_line":"    for greenthread in greenthreads:"},{"line_number":1616,"context_line":"        found \u003d greenthread.wait()"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_b7ed7d8d","line":1613,"range":{"start_line":1613,"start_character":37,"end_line":1613,"end_character":64},"updated":"2017-03-07 22:03:55.000000000","message":"QUESTION: Should I change this to not join with instance_extra and instead count instance.vcpus and instance.memory_mb? It doesn\u0027t help us in the resize case but obviously avoids a join.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a9ca7e845b89120c60f1cd4ad81a114883f5d400","unresolved":false,"context_lines":[{"line_number":1610,"context_line":"            context_copy \u003d copy.copy(context)"},{"line_number":1611,"context_line":"            greenthreads.append(utils.spawn("},{"line_number":1612,"context_line":"                objects.InstanceList.get_by_filters,"},{"line_number":1613,"context_line":"                context_copy, filters, expected_attrs\u003d[\u0027flavor\u0027]))"},{"line_number":1614,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1615,"context_line":"    for greenthread in greenthreads:"},{"line_number":1616,"context_line":"        found \u003d greenthread.wait()"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_1a10054e","line":1613,"range":{"start_line":1613,"start_character":37,"end_line":1613,"end_character":64},"in_reply_to":"9a30ddce_593b7438","updated":"2017-03-08 18:50:08.000000000","message":"The max operation would help in the case of downsizes with no race. The concern I had was with races (though I doubt races with resizes happen much in practice). I thought about it when I was going through and adding the \"recheck quota after resource consumed and delete if overquota\" code and saw we have a gap for resizes. I\u0027m okay with that though, after hearing you and johnthetubaguy\u0027s thoughts.\n\nThat said, I don\u0027t mind changing this to count instance attrs instead of flavor, if you think it\u0027s worth it. I made it count flavor since at the time, we were thinking of this approach as being more long-term (outliving the instance attrs, I guess?). Now we are thinking of replacing with placement, near-term.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6193982379a2234e778397311f756a3e80ae08b2","unresolved":false,"context_lines":[{"line_number":1610,"context_line":"            context_copy \u003d copy.copy(context)"},{"line_number":1611,"context_line":"            greenthreads.append(utils.spawn("},{"line_number":1612,"context_line":"                objects.InstanceList.get_by_filters,"},{"line_number":1613,"context_line":"                context_copy, filters, expected_attrs\u003d[\u0027flavor\u0027]))"},{"line_number":1614,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1615,"context_line":"    for greenthread in greenthreads:"},{"line_number":1616,"context_line":"        found \u003d greenthread.wait()"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_593b7438","line":1613,"range":{"start_line":1613,"start_character":37,"end_line":1613,"end_character":64},"in_reply_to":"9a30ddce_b7ed7d8d","updated":"2017-03-08 15:55:24.000000000","message":"It also means we end up deserializing a bunch of json. If we do this then we can do that max operation I describe earlier right? Probably worth it for that reason no?","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6193982379a2234e778397311f756a3e80ae08b2","unresolved":false,"context_lines":[{"line_number":1613,"context_line":"                context_copy, filters, expected_attrs\u003d[\u0027flavor\u0027]))"},{"line_number":1614,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1615,"context_line":"    for greenthread in greenthreads:"},{"line_number":1616,"context_line":"        found \u003d greenthread.wait()"},{"line_number":1617,"context_line":"        instances \u003d instances + found"},{"line_number":1618,"context_line":"    total_counts \u003d {\u0027project\u0027: {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}}"},{"line_number":1619,"context_line":"    if user_id:"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a30ddce_b912b0b2","line":1616,"updated":"2017-03-08 15:55:24.000000000","message":"Yay for parallel cells optimized queries.","commit_id":"2eb19fdf36213673978e440215dd7d193ddd55a0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6e62f9d4ff27d9961cb46c0faf9d89b2fc1955c3","unresolved":false,"context_lines":[{"line_number":157,"context_line":"    def _get_usages(self, context, resources, project_id, user_id\u003dNone):"},{"line_number":158,"context_line":"        key \u003d \u0027user\u0027 if user_id else \u0027project\u0027"},{"line_number":159,"context_line":"        usages \u003d {}"},{"line_number":160,"context_line":"        for resource in resources.values():"},{"line_number":161,"context_line":"            if resource.name in usages:"},{"line_number":162,"context_line":"                continue"},{"line_number":163,"context_line":"            if resource.name in (\u0027instances\u0027, \u0027cores\u0027, \u0027ram\u0027):"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_e230ffea","line":160,"updated":"2017-03-16 00:30:56.000000000","message":"As we discussed, this probably needs to be generalized to handle things that aren\u0027t Countable in here (i.e. don\u0027t have a .count() method). Then we can use this earlier and have it just work as things get converted to countable.","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6e62f9d4ff27d9961cb46c0faf9d89b2fc1955c3","unresolved":false,"context_lines":[{"line_number":165,"context_line":"                for res in (\u0027instances\u0027, \u0027cores\u0027, \u0027ram\u0027):"},{"line_number":166,"context_line":"                    count \u003d counts[key][res]"},{"line_number":167,"context_line":"                    usages[res] \u003d {\u0027in_use\u0027: count}"},{"line_number":168,"context_line":"            elif resource.name in (\u0027security_groups\u0027, \u0027server_groups\u0027):"},{"line_number":169,"context_line":"                counts \u003d resource.count(context, project_id, user_id\u003duser_id)"},{"line_number":170,"context_line":"                usages[resource.name] \u003d {\u0027in_use\u0027: counts[key][resource.name]}"},{"line_number":171,"context_line":"            elif resource.name in (\u0027floating_ips\u0027, \u0027fixed_ips\u0027, \u0027networks\u0027):"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_a20fe72c","line":168,"updated":"2017-03-16 00:30:56.000000000","message":"How is this different from the instances,cores,ram bit above? Looks the same to me.","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6e62f9d4ff27d9961cb46c0faf9d89b2fc1955c3","unresolved":false,"context_lines":[{"line_number":169,"context_line":"                counts \u003d resource.count(context, project_id, user_id\u003duser_id)"},{"line_number":170,"context_line":"                usages[resource.name] \u003d {\u0027in_use\u0027: counts[key][resource.name]}"},{"line_number":171,"context_line":"            elif resource.name in (\u0027floating_ips\u0027, \u0027fixed_ips\u0027, \u0027networks\u0027):"},{"line_number":172,"context_line":"                count \u003d resource.count(context, project_id)"},{"line_number":173,"context_line":"                usages[resource.name] \u003d {\u0027in_use\u0027: count}"},{"line_number":174,"context_line":"            else:"},{"line_number":175,"context_line":"                # The other per user resources are special cases whose"}],"source_content_type":"text/x-python","patch_set":22,"id":"3a1ff146_4221d39e","line":172,"updated":"2017-03-16 00:30:56.000000000","message":"This could probably be the same as the above too, maybe just ignoring the project and user?","commit_id":"1411f183560a1e0af4f8c233c61f32dc86da3132"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5606207ded920bfb422b28c78a850fa88a7747ed","unresolved":false,"context_lines":[{"line_number":1890,"context_line":"    # placement for cores/ram and InstanceMappings for instances (once we are"},{"line_number":1891,"context_line":"    # deleting InstanceMappings when we delete instances)."},{"line_number":1892,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id}"},{"line_number":1893,"context_line":"    results \u003d nova_context.scatter_gather_all_cells("},{"line_number":1894,"context_line":"        context, objects.InstanceList.get_by_filters, filters)"},{"line_number":1895,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1896,"context_line":"    for cell_uuid, result in results.items():"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_181bca1b","line":1893,"range":{"start_line":1893,"start_character":27,"end_line":1893,"end_character":51},"updated":"2017-07-12 22:42:16.000000000","message":"OK this is all cells because you could have instances in cell0 which would be in ERROR state and those should still count against your quota usage until you delete them.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e3f44594aec3526596ca0d9237ce5d7cdb027496","unresolved":false,"context_lines":[{"line_number":1891,"context_line":"    # deleting InstanceMappings when we delete instances)."},{"line_number":1892,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id}"},{"line_number":1893,"context_line":"    results \u003d nova_context.scatter_gather_all_cells("},{"line_number":1894,"context_line":"        context, objects.InstanceList.get_by_filters, filters)"},{"line_number":1895,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1896,"context_line":"    for cell_uuid, result in results.items():"},{"line_number":1897,"context_line":"        if result not in (nova_context.did_not_respond_sentinel,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_3d606ca5","line":1894,"range":{"start_line":1894,"start_character":17,"end_line":1894,"end_character":52},"updated":"2017-07-11 20:55:09.000000000","message":"We don\u0027t need to join on any fancy tables, right? We should pass expected_attrs\u003d[] to make this as fast as possible and avoid unnecessary joins. Because if it\u0027s None it defaults to this in the DB API:\n\ncolumns_to_join_new \u003d [\u0027info_cache\u0027, \u0027security_groups\u0027]","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d5c3cbf752563282a9884af8ab24a63d0f655b91","unresolved":false,"context_lines":[{"line_number":1891,"context_line":"    # deleting InstanceMappings when we delete instances)."},{"line_number":1892,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id}"},{"line_number":1893,"context_line":"    results \u003d nova_context.scatter_gather_all_cells("},{"line_number":1894,"context_line":"        context, objects.InstanceList.get_by_filters, filters)"},{"line_number":1895,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1896,"context_line":"    for cell_uuid, result in results.items():"},{"line_number":1897,"context_line":"        if result not in (nova_context.did_not_respond_sentinel,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_e0248bb1","line":1894,"range":{"start_line":1894,"start_character":17,"end_line":1894,"end_character":52},"in_reply_to":"3f1d235d_3d606ca5","updated":"2017-07-11 21:20:44.000000000","message":"If we\u0027re not looking at flavor, then yeah. Definitely don\u0027t need secgroups and info_cache for this.\n\nI thought we were going to try to count these in SQL? I would expect that two queries, one for project_id and one for user_id (or maybe there is a fancy double grouping we could do) would be more efficient than pulling the whole instance objects.\n\nI know we\u0027re going to count this with placement at some point, but...","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1891,"context_line":"    # deleting InstanceMappings when we delete instances)."},{"line_number":1892,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id}"},{"line_number":1893,"context_line":"    results \u003d nova_context.scatter_gather_all_cells("},{"line_number":1894,"context_line":"        context, objects.InstanceList.get_by_filters, filters)"},{"line_number":1895,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1896,"context_line":"    for cell_uuid, result in results.items():"},{"line_number":1897,"context_line":"        if result not in (nova_context.did_not_respond_sentinel,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_b5f92c81","line":1894,"range":{"start_line":1894,"start_character":17,"end_line":1894,"end_character":52},"in_reply_to":"3f1d235d_3d606ca5","updated":"2017-07-12 02:27:38.000000000","message":"Oh, okay. I didn\u0027t remember that get_by_filters() adds its own columns to join.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1891,"context_line":"    # deleting InstanceMappings when we delete instances)."},{"line_number":1892,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id}"},{"line_number":1893,"context_line":"    results \u003d nova_context.scatter_gather_all_cells("},{"line_number":1894,"context_line":"        context, objects.InstanceList.get_by_filters, filters)"},{"line_number":1895,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1896,"context_line":"    for cell_uuid, result in results.items():"},{"line_number":1897,"context_line":"        if result not in (nova_context.did_not_respond_sentinel,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_95399040","line":1894,"range":{"start_line":1894,"start_character":17,"end_line":1894,"end_character":52},"in_reply_to":"3f1d235d_e0248bb1","updated":"2017-07-12 02:27:38.000000000","message":"Oh ... yeah, this is remnant of the fact that when I first wrote this, I was counting stuff in the instance.flavor. Though maybe that could have been counted in SQL too. Will fix.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5606207ded920bfb422b28c78a850fa88a7747ed","unresolved":false,"context_lines":[{"line_number":1892,"context_line":"    filters \u003d {\u0027deleted\u0027: False, \u0027project_id\u0027: project_id}"},{"line_number":1893,"context_line":"    results \u003d nova_context.scatter_gather_all_cells("},{"line_number":1894,"context_line":"        context, objects.InstanceList.get_by_filters, filters)"},{"line_number":1895,"context_line":"    instances \u003d objects.InstanceList(objects\u003d[])"},{"line_number":1896,"context_line":"    for cell_uuid, result in results.items():"},{"line_number":1897,"context_line":"        if result not in (nova_context.did_not_respond_sentinel,"},{"line_number":1898,"context_line":"                          nova_context.raised_exception_sentinel):"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_f817eeec","line":1895,"range":{"start_line":1895,"start_character":37,"end_line":1895,"end_character":47},"updated":"2017-07-12 22:42:16.000000000","message":"nit: this is redundant, pretty sure you don\u0027t need to explicitly set this for an empty object list.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d5c3cbf752563282a9884af8ab24a63d0f655b91","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"    if user_id:"},{"line_number":1902,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1903,"context_line":"    # NOTE(melwitt): Should we count video ram here too? We won\u0027t be able to"},{"line_number":1904,"context_line":"    # count it with placement unless placement is tracking video ram."},{"line_number":1905,"context_line":"    for instance in instances:"},{"line_number":1906,"context_line":"        instance_count \u003d 1"},{"line_number":1907,"context_line":"        cores_count \u003d instance.vcpus"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_404d7f70","line":1904,"updated":"2017-07-11 21:20:44.000000000","message":"Where is video ram tracked? Isn\u0027t that an extra_spec on flavor for certain hypervisors or something?","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c372894f82aa7d0c557a716c644644677d03cfb0","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"    if user_id:"},{"line_number":1902,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1903,"context_line":"    # NOTE(melwitt): Should we count video ram here too? We won\u0027t be able to"},{"line_number":1904,"context_line":"    # count it with placement unless placement is tracking video ram."},{"line_number":1905,"context_line":"    for instance in instances:"},{"line_number":1906,"context_line":"        instance_count \u003d 1"},{"line_number":1907,"context_line":"        cores_count \u003d instance.vcpus"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_97dc4875","line":1904,"in_reply_to":"1f1a1f67_d8b792d8","updated":"2017-07-13 00:02:37.000000000","message":"Once you rebase on top of https://review.openstack.org/#/c/483133/ we\u0027ll be golden. And then let\u0027s never speak of that thing again.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6d2ad7fba2955c0c3eefd5196323310bbc827fb9","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"    if user_id:"},{"line_number":1902,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1903,"context_line":"    # NOTE(melwitt): Should we count video ram here too? We won\u0027t be able to"},{"line_number":1904,"context_line":"    # count it with placement unless placement is tracking video ram."},{"line_number":1905,"context_line":"    for instance in instances:"},{"line_number":1906,"context_line":"        instance_count \u003d 1"},{"line_number":1907,"context_line":"        cores_count \u003d instance.vcpus"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_ec112406","line":1904,"in_reply_to":"3f1d235d_2cd5dc43","updated":"2017-07-12 00:25:43.000000000","message":"Only other place this shows up is in the libvirt driver when creating the guest:\n\nhttps://github.com/openstack/nova/blob/465bbccac94a2e62cad0598f8894ac4e9e86ec47/nova/virt/libvirt/driver.py#L4338\n\nSo nothing tracked in claims for an overhead calculation or anything, of course. So yeah, latent bug.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1b9407c4ec98fbb51c35eaf00d891b1245fef153","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"    if user_id:"},{"line_number":1902,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1903,"context_line":"    # NOTE(melwitt): Should we count video ram here too? We won\u0027t be able to"},{"line_number":1904,"context_line":"    # count it with placement unless placement is tracking video ram."},{"line_number":1905,"context_line":"    for instance in instances:"},{"line_number":1906,"context_line":"        instance_count \u003d 1"},{"line_number":1907,"context_line":"        cores_count \u003d instance.vcpus"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_2cd5dc43","line":1904,"in_reply_to":"3f1d235d_404d7f70","updated":"2017-07-12 00:22:58.000000000","message":"It\u0027s an extra spec on flavors, yeah:\n\nhttps://review.openstack.org/#/c/416521/58/nova/compute/utils.py@716\n\nWe check it on create ^ and delete:\n\nhttps://review.openstack.org/#/c/416521/58/nova/compute/api.py@a2133\n\nBut I don\u0027t think we actually account for that during placement decisions, or claims on the compute, which is probably just a super latent bug.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5606207ded920bfb422b28c78a850fa88a7747ed","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"    if user_id:"},{"line_number":1902,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1903,"context_line":"    # NOTE(melwitt): Should we count video ram here too? We won\u0027t be able to"},{"line_number":1904,"context_line":"    # count it with placement unless placement is tracking video ram."},{"line_number":1905,"context_line":"    for instance in instances:"},{"line_number":1906,"context_line":"        instance_count \u003d 1"},{"line_number":1907,"context_line":"        cores_count \u003d instance.vcpus"}],"source_content_type":"text/x-python","patch_set":58,"id":"1f1a1f67_d8b792d8","line":1904,"in_reply_to":"3f1d235d_752154fb","updated":"2017-07-12 22:42:16.000000000","message":"I\u0027ve reported a bug to track it for claims/allocations so we don\u0027t lose the issue, although it will just get lost in the bug tracker.\n\nhttps://bugs.launchpad.net/nova/+bug/1704012\n\nAs for this change, the video ram is used to check from the API:\n\nhttps://review.openstack.org/#/c/416521/58/nova/compute/utils.py@716\n\nSo couldn\u0027t that actually slip through here now since you\u0027re not accounting for it when counting the instance ram usage? Granted we\u0027d then have to load the instance.flavor.extra_specs too for this calculation, which sucks for performance for something that is probably used in 5% of cases...maybe that\u0027s solved long-term by tracking it via the MEMORY_MB allocation in placement.\n\nBefore counting quotas, we\u0027d create a reservation to increment usage for the flavor.memory_mb + video_ram and then on delete we\u0027d decrement usage for the same thing, so it was actually part of the usage. Now with counting quotas, we\u0027re going to check against flavor.memory_mb + video_ram but not actually include that in the current usage, which seems wrong.\n\nIf we don\u0027t think many people are using this, and we want to avoid the performance penalty by pulling the instance.flavor.extra_specs to account for video ram, we could doc this as a known limitation saying it will have to be resolved when we start accounting for video ram in placement with the MEMORY_MB allocation and then this code is using placement rather than the cell db.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47e5ecb428ea6c6711306252cd0539f52f06f976","unresolved":false,"context_lines":[{"line_number":1901,"context_line":"    if user_id:"},{"line_number":1902,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1903,"context_line":"    # NOTE(melwitt): Should we count video ram here too? We won\u0027t be able to"},{"line_number":1904,"context_line":"    # count it with placement unless placement is tracking video ram."},{"line_number":1905,"context_line":"    for instance in instances:"},{"line_number":1906,"context_line":"        instance_count \u003d 1"},{"line_number":1907,"context_line":"        cores_count \u003d instance.vcpus"}],"source_content_type":"text/x-python","patch_set":58,"id":"3f1d235d_752154fb","line":1904,"in_reply_to":"3f1d235d_ec112406","updated":"2017-07-12 02:27:38.000000000","message":"Yeah ... someone has recently been fixing bugs where video ram was only partially accounted for in quotas (it was something like, ram usage was increased + video ram upon create, but video ram wasn\u0027t subtracted upon delete). I was thinking we should just ignore video ram across the board for now.","commit_id":"c4c1e609cd60df6e324110635b32aee186947fed"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"126c60d47be0ed508f93f654714c7b5a30c8cd57","unresolved":false,"context_lines":[{"line_number":1892,"context_line":"    results \u003d nova_context.scatter_gather_all_cells("},{"line_number":1893,"context_line":"        context, objects.InstanceList.get_counts, project_id, user_id\u003duser_id)"},{"line_number":1894,"context_line":"    total_counts \u003d {\u0027project\u0027: {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}}"},{"line_number":1895,"context_line":"    if user_id:"},{"line_number":1896,"context_line":"        total_counts[\u0027user\u0027] \u003d {\u0027instances\u0027: 0, \u0027cores\u0027: 0, \u0027ram\u0027: 0}"},{"line_number":1897,"context_line":"    for cell_uuid, result in results.items():"},{"line_number":1898,"context_line":"        if result not in (nova_context.did_not_respond_sentinel,"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_8ca6c90e","line":1895,"range":{"start_line":1895,"start_character":4,"end_line":1895,"end_character":15},"updated":"2017-07-19 01:58:18.000000000","message":"Test coverage report is saying there isn\u0027t a test where this is None. Could be handled later in a follow up.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cfcb41884da53a7ecc1caea68ae5d22df58f71ff","unresolved":false,"context_lines":[{"line_number":1692,"context_line":"                                     \u0027nodename\u0027: \u0027fake-nodename\u0027,"},{"line_number":1693,"context_line":"                                     \u0027limits\u0027: None}]"},{"line_number":1694,"context_line":"        # Simulate a race where the first check passes and the recheck fails."},{"line_number":1695,"context_line":"        # First check occurs in compute/api."},{"line_number":1696,"context_line":"        fake_quotas \u003d {\u0027instances\u0027: 5, \u0027cores\u0027: 10, \u0027ram\u0027: 4096}"},{"line_number":1697,"context_line":"        fake_headroom \u003d {\u0027instances\u0027: 5, \u0027cores\u0027: 10, \u0027ram\u0027: 4096}"},{"line_number":1698,"context_line":"        fake_usages \u003d {\u0027instances\u0027: 5, \u0027cores\u0027: 10, \u0027ram\u0027: 4096}"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_ec077dc9","line":1695,"range":{"start_line":1695,"start_character":8,"end_line":1695,"end_character":44},"updated":"2017-07-19 02:02:06.000000000","message":"I don\u0027t get this - how do we get to the compute API from here?","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"abe5115ac642336eaa0efb6dc4828e20fe0d61b2","unresolved":false,"context_lines":[{"line_number":1692,"context_line":"                                     \u0027nodename\u0027: \u0027fake-nodename\u0027,"},{"line_number":1693,"context_line":"                                     \u0027limits\u0027: None}]"},{"line_number":1694,"context_line":"        # Simulate a race where the first check passes and the recheck fails."},{"line_number":1695,"context_line":"        # First check occurs in compute/api."},{"line_number":1696,"context_line":"        fake_quotas \u003d {\u0027instances\u0027: 5, \u0027cores\u0027: 10, \u0027ram\u0027: 4096}"},{"line_number":1697,"context_line":"        fake_headroom \u003d {\u0027instances\u0027: 5, \u0027cores\u0027: 10, \u0027ram\u0027: 4096}"},{"line_number":1698,"context_line":"        fake_usages \u003d {\u0027instances\u0027: 5, \u0027cores\u0027: 10, \u0027ram\u0027: 4096}"}],"source_content_type":"text/x-python","patch_set":63,"id":"1f1a1f67_2c1a1516","line":1695,"range":{"start_line":1695,"start_character":8,"end_line":1695,"end_character":44},"in_reply_to":"1f1a1f67_ec077dc9","updated":"2017-07-19 02:06:55.000000000","message":"Maybe you\u0027re just saying, we\u0027d check in the API during create, which passes, and this is the recheck in conductor, which fails.","commit_id":"5c90b25e49d47deb7dc6695333d9d5e46efe8665"}]}