)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f44bdca36507a92abf6432667cae71056a8c9430","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adds logic to call the placement API\u0027s PUT /allocations/{consumer_uuid}"},{"line_number":10,"context_line":"when selecting hosts in the filter scheduler\u0027s _schedule() method."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"We only attempt the claim of resources IFF the scheduler driver uses"},{"line_number":13,"context_line":"allocation candidates (i.e. it isn\u0027t the caching scheduler) and the"},{"line_number":14,"context_line":"conductor has passed in a list of instance UUIDs (otherwise, there\u0027s no"},{"line_number":15,"context_line":"way to allocate in the placement API)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1f1a1f67_4a7cf070","line":12,"range":{"start_line":12,"start_character":39,"end_line":12,"end_character":42},"updated":"2017-07-19 20:52:44.000000000","message":"You might want to spell that out for those who might not be familiar with this acronym.","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adds logic to call the placement API\u0027s PUT /allocations/{consumer_uuid}"},{"line_number":10,"context_line":"when selecting hosts in the filter scheduler\u0027s _schedule() method."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"We only attempt the claim of resources IFF the scheduler driver uses"},{"line_number":13,"context_line":"allocation candidates (i.e. it isn\u0027t the caching scheduler) and the"},{"line_number":14,"context_line":"conductor has passed in a list of instance UUIDs (otherwise, there\u0027s no"},{"line_number":15,"context_line":"way to allocate in the placement API)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1f1a1f67_e6206c9e","line":12,"range":{"start_line":12,"start_character":39,"end_line":12,"end_character":42},"in_reply_to":"1f1a1f67_4a7cf070","updated":"2017-07-20 23:14:52.000000000","message":"Done","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"054b4fa0a9d7ef4db22af288613a94797f4b9374","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"We only attempt the claim of resources if and only if the scheduler"},{"line_number":13,"context_line":"driver uses allocation candidates (i.e. it isn\u0027t the caching scheduler)"},{"line_number":14,"context_line":"and the conductor has passed in a list of instance UUIDs (otherwise,"},{"line_number":15,"context_line":"there\u0027s no way to allocate in the placement API)."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ifc5cf482209e4f6f4e3e39b24389bd3563d86444"},{"line_number":18,"context_line":"blueprint: placement-claims"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"ff346bd7_9a2275b8","line":15,"range":{"start_line":14,"start_character":0,"end_line":15,"end_character":49},"updated":"2017-07-27 21:42:00.000000000","message":"OK confirmed that conductor does this for everything we expect it to:\n\n1. build (new instance(s) create)\n2. unshelve\n3. migrate (cold and resize)\n4. live migrate\n5. rebuild (+evacuate)\n\nFor that last one, rebuild to the same host, will we double allocate?\n\nhttps://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L770","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"84596cd6979201b27ff865bcab872d7fe064fe75","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"We only attempt the claim of resources if and only if the scheduler"},{"line_number":13,"context_line":"driver uses allocation candidates (i.e. it isn\u0027t the caching scheduler)"},{"line_number":14,"context_line":"and the conductor has passed in a list of instance UUIDs (otherwise,"},{"line_number":15,"context_line":"there\u0027s no way to allocate in the placement API)."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ifc5cf482209e4f6f4e3e39b24389bd3563d86444"},{"line_number":18,"context_line":"blueprint: placement-claims"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"ff346bd7_7d346bde","line":15,"range":{"start_line":14,"start_character":0,"end_line":15,"end_character":49},"in_reply_to":"ff346bd7_9a2275b8","updated":"2017-07-27 21:44:58.000000000","message":"We\u0027re OK for rebuild, we don\u0027t call select_destinations in that case because the compute API calls conductor and passes host\u003dinstance.host:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L2993\n\nAnd only if there isn\u0027t a host provided does conductor ask for one from the scheduler:\n\nhttps://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L748","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"}],"nova/scheduler/filter_scheduler.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"68799d062d779c4c354c9a7204c765119c1ef0f4","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if not sorted_hosts:"},{"line_number":146,"context_line":"                break"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if instance_uuids is None or not self.USES_ALLOCATION_CANDIDATES:"},{"line_number":149,"context_line":"                # Unfortunately, we still need to deal with older conductors"},{"line_number":150,"context_line":"                # that may not be passing in a list of instance_uuids. In those"},{"line_number":151,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_ae4b2b57","line":148,"range":{"start_line":148,"start_character":15,"end_line":148,"end_character":29},"updated":"2017-07-14 15:12:16.000000000","message":"I guess we always can get instance_uuids from the conductor. And conductor is always the first component upgrade in the cluster.","commit_id":"68335c102fd47ac15b447b97bfff24334dcc4c9b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1222a9fe6e20c32b31f9647bfc6fa02f600d1a45","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if not sorted_hosts:"},{"line_number":146,"context_line":"                break"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if instance_uuids is None or not self.USES_ALLOCATION_CANDIDATES:"},{"line_number":149,"context_line":"                # Unfortunately, we still need to deal with older conductors"},{"line_number":150,"context_line":"                # that may not be passing in a list of instance_uuids. In those"},{"line_number":151,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_ae932bfa","line":148,"range":{"start_line":148,"start_character":15,"end_line":148,"end_character":29},"in_reply_to":"1f1a1f67_ae4b2b57","updated":"2017-07-14 15:20:31.000000000","message":"Apparently not :) Older conductor clients do not pass an instance_uuids parameter :(","commit_id":"68335c102fd47ac15b447b97bfff24334dcc4c9b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8a4efe5ee8a76aeb68b84d99641e6ccfb203917c","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if not sorted_hosts:"},{"line_number":146,"context_line":"                break"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if instance_uuids is None or not self.USES_ALLOCATION_CANDIDATES:"},{"line_number":149,"context_line":"                # Unfortunately, we still need to deal with older conductors"},{"line_number":150,"context_line":"                # that may not be passing in a list of instance_uuids. In those"},{"line_number":151,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_8b1def10","line":148,"range":{"start_line":148,"start_character":15,"end_line":148,"end_character":29},"in_reply_to":"1f1a1f67_ae932bfa","updated":"2017-07-15 05:01:18.000000000","message":"ok...\n\nhttps://docs.openstack.org/nova/latest/upgrade.html#rolling-upgrade-process\n\n\"It is safest to start nova-conductor first and nova-api last.\"\n\nthat is what we suggested. But yes, scheduler can newer than conductor. I thought we don\u0027t suggest anyone that.","commit_id":"68335c102fd47ac15b447b97bfff24334dcc4c9b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a02e04456249c9f218df30e2fa610f00e2174adb","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if not hosts:"},{"line_number":146,"context_line":"                break"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if instance_uuids is None or not self.USES_ALLOCATION_CANDIDATES:"},{"line_number":149,"context_line":"                # Unfortunately, we still need to deal with older conductors"},{"line_number":150,"context_line":"                # that may not be passing in a list of instance_uuids. In those"},{"line_number":151,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_cb7774d4","line":148,"updated":"2017-07-14 17:33:45.000000000","message":"inheritance, what fun it is","commit_id":"5f3f92f9828b40fce3c1b7d05cfe0a6df5967ce2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"18c589a437ee17461003f8f7e29a8624daa464bf","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        supplied request_spec."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        The hosts will have had allocations made against them in the placement"},{"line_number":52,"context_line":"        API."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        :param context: The RequestContext object"},{"line_number":55,"context_line":"        :param spec_obj: The RequestSpec object"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_013ca82c","line":52,"updated":"2017-07-19 17:06:17.000000000","message":"interesting docstring","commit_id":"529b1239318e8b7ace6fb956415aa4ce3e11ad6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"18c589a437ee17461003f8f7e29a8624daa464bf","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            if not hosts:"},{"line_number":146,"context_line":"                break"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if instance_uuids is None or not self.USES_ALLOCATION_CANDIDATES:"},{"line_number":149,"context_line":"                # Unfortunately, we still need to deal with older conductors"},{"line_number":150,"context_line":"                # that may not be passing in a list of instance_uuids. In those"},{"line_number":151,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_a17bb4f2","line":148,"range":{"start_line":148,"start_character":41,"end_line":148,"end_character":77},"updated":"2017-07-19 17:06:17.000000000","message":"that\u0027s very unfortunate we still need to verify this attribute because of the CachingScheduler... :(\nMaybe a comment is worth it?","commit_id":"529b1239318e8b7ace6fb956415aa4ce3e11ad6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"18c589a437ee17461003f8f7e29a8624daa464bf","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"},{"line_number":152,"context_line":"                # have instance UUIDs to claim with, so we just grab the first"},{"line_number":153,"context_line":"                # host in the list of sorted hosts."},{"line_number":154,"context_line":"                claimed_host \u003d hosts[0]"},{"line_number":155,"context_line":"            else:"},{"line_number":156,"context_line":"                instance_uuid \u003d instance_uuids[num]"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_e14d2cc4","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":39},"updated":"2017-07-19 17:06:17.000000000","message":"it\u0027s technically not \"claimed\", but okay with the comment","commit_id":"529b1239318e8b7ace6fb956415aa4ce3e11ad6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"18c589a437ee17461003f8f7e29a8624daa464bf","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                LOG.debug(\"Unable to successfully claim against any host.\")"},{"line_number":179,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":180,"context_line":"                return []"},{"line_number":181,"context_line":"            elif not self.USES_ALLOCATION_CANDIDATES:"},{"line_number":182,"context_line":"                # We\u0027re not claiming anything so don\u0027t bother doing any cleanup"},{"line_number":183,"context_line":"                pass"},{"line_number":184,"context_line":"            elif instance_uuids is not None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_c1523035","line":181,"updated":"2017-07-19 17:06:17.000000000","message":"nit: if we were just doing beofre L148\n\n  if not self.USES_ALLOCATION_CANDIDATES:\n    claimed_host \u003d hosts[0]\n  else:\n   \u003call of that\u003e\n\nit would surely add one indent but it would clarify that all of that is because that driver supports placement ?","commit_id":"529b1239318e8b7ace6fb956415aa4ce3e11ad6a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3e12bb7a91106d2fd4c55be1a09a3aec53922001","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                LOG.debug(\"Unable to successfully claim against any host.\")"},{"line_number":179,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":180,"context_line":"                return []"},{"line_number":181,"context_line":"            elif not self.USES_ALLOCATION_CANDIDATES:"},{"line_number":182,"context_line":"                # We\u0027re not claiming anything so don\u0027t bother doing any cleanup"},{"line_number":183,"context_line":"                pass"},{"line_number":184,"context_line":"            elif instance_uuids is not None:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_307128a9","line":181,"in_reply_to":"1f1a1f67_c1523035","updated":"2017-07-19 19:02:06.000000000","message":"yep, good point. will do.","commit_id":"529b1239318e8b7ace6fb956415aa4ce3e11ad6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"18c589a437ee17461003f8f7e29a8624daa464bf","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"            # Now consume the resources so the filter/weights will change for"},{"line_number":191,"context_line":"            # the next instance."},{"line_number":192,"context_line":"            claimed_host.consume_from_request(spec_obj)"},{"line_number":193,"context_line":"            if spec_obj.instance_group is not None:"},{"line_number":194,"context_line":"                spec_obj.instance_group.hosts.append(claimed_host.host)"},{"line_number":195,"context_line":"                # hosts has to be not part of the updates when saving"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_a4018207","line":192,"updated":"2017-07-19 17:06:17.000000000","message":"I have a thought, couldn\u0027we try to claim the allocation in that synchronized section ?\nI mean, instead of calling _claim_resources at L167, we could just try to call claimed_host.consume_from_request() which would do the placement call by itself\n\nIn case something goes wrong, we could throw an exception from consume_from_request which would then be catched in the loop of L159 ?","commit_id":"529b1239318e8b7ace6fb956415aa4ce3e11ad6a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3e12bb7a91106d2fd4c55be1a09a3aec53922001","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"            # Now consume the resources so the filter/weights will change for"},{"line_number":191,"context_line":"            # the next instance."},{"line_number":192,"context_line":"            claimed_host.consume_from_request(spec_obj)"},{"line_number":193,"context_line":"            if spec_obj.instance_group is not None:"},{"line_number":194,"context_line":"                spec_obj.instance_group.hosts.append(claimed_host.host)"},{"line_number":195,"context_line":"                # hosts has to be not part of the updates when saving"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f1a1f67_10762c71","line":192,"in_reply_to":"1f1a1f67_a4018207","updated":"2017-07-19 19:02:06.000000000","message":"That means I would have to pass a reportclient object along with the alloc_reqs_by_rp_uuid dict to the HostState.consume_from_request() method. I\u0027m trying to avoid that kind of coupling of state and claim logic as much as possible.","commit_id":"529b1239318e8b7ace6fb956415aa4ce3e11ad6a"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f44bdca36507a92abf6432667cae71056a8c9430","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        \"\"\"Returns a list of hosts that meet the required specs, ordered by"},{"line_number":115,"context_line":"        their fitness."},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        The hosts will have had allocations made against them in the placement"},{"line_number":118,"context_line":"        API."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        :param context: The RequestContext object"},{"line_number":121,"context_line":"        :param spec_obj: The RequestSpec object"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_aa4dc43b","line":118,"range":{"start_line":117,"start_character":8,"end_line":118,"end_character":12},"updated":"2017-07-19 20:52:44.000000000","message":"Awkward wording. Maybe: \"These hosts will have already had their resources claimed in Placement.\"","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        \"\"\"Returns a list of hosts that meet the required specs, ordered by"},{"line_number":115,"context_line":"        their fitness."},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        The hosts will have had allocations made against them in the placement"},{"line_number":118,"context_line":"        API."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        :param context: The RequestContext object"},{"line_number":121,"context_line":"        :param spec_obj: The RequestSpec object"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_a66834ba","line":118,"range":{"start_line":117,"start_character":8,"end_line":118,"end_character":12},"in_reply_to":"1f1a1f67_aa4dc43b","updated":"2017-07-20 23:14:52.000000000","message":"Done","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a93f61fbd1c0104adfe6f8783a83961f851d5609","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                # looking for an allocation request that contains that host\u0027s"},{"line_number":187,"context_line":"                # resource provider UUID"},{"line_number":188,"context_line":"                claimed_host \u003d None"},{"line_number":189,"context_line":"                for host in hosts:"},{"line_number":190,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":191,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":192,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_e0073418","line":189,"updated":"2017-07-20 21:27:15.000000000","message":"As mentioned in the nova meeting, why don\u0027t we limit the number of retries here based on the config? Do we want to potentially retry 1000 times?","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                # looking for an allocation request that contains that host\u0027s"},{"line_number":187,"context_line":"                # resource provider UUID"},{"line_number":188,"context_line":"                claimed_host \u003d None"},{"line_number":189,"context_line":"                for host in hosts:"},{"line_number":190,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":191,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":192,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_465f200e","line":189,"in_reply_to":"1f1a1f67_803c00c0","updated":"2017-07-20 23:14:52.000000000","message":"Done","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e06e31e9808129550bdf5f1fd27a43d37feb6ad","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                # looking for an allocation request that contains that host\u0027s"},{"line_number":187,"context_line":"                # resource provider UUID"},{"line_number":188,"context_line":"                claimed_host \u003d None"},{"line_number":189,"context_line":"                for host in hosts:"},{"line_number":190,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":191,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":192,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_803c00c0","line":189,"in_reply_to":"1f1a1f67_e0073418","updated":"2017-07-20 21:31:05.000000000","message":"Let\u0027s do a release note for this.","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a70e54cb2e74bb1cbc87ec29fcdbeab2420f560e","unresolved":false,"context_lines":[{"line_number":188,"context_line":"                claimed_host \u003d None"},{"line_number":189,"context_line":"                for host in hosts:"},{"line_number":190,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":191,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":192,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""},{"line_number":193,"context_line":"                                  \"allocation requests. Skipping.\", cn_uuid)"},{"line_number":194,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_4eb0698c","line":191,"range":{"start_line":191,"start_character":38,"end_line":191,"end_character":59},"updated":"2017-07-20 14:23:07.000000000","message":"This can be None, as gibi pointed out in:\n\nhttps://review.openstack.org/#/c/483565/4/nova/scheduler/manager.py","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":188,"context_line":"                claimed_host \u003d None"},{"line_number":189,"context_line":"                for host in hosts:"},{"line_number":190,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":191,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":192,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""},{"line_number":193,"context_line":"                                  \"allocation requests. Skipping.\", cn_uuid)"},{"line_number":194,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_a6495433","line":191,"range":{"start_line":191,"start_character":38,"end_line":191,"end_character":59},"in_reply_to":"1f1a1f67_39269518","updated":"2017-07-20 23:14:52.000000000","message":"Done","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7e063544471c39d76022b5c066337a9c24956da1","unresolved":false,"context_lines":[{"line_number":188,"context_line":"                claimed_host \u003d None"},{"line_number":189,"context_line":"                for host in hosts:"},{"line_number":190,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":191,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":192,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""},{"line_number":193,"context_line":"                                  \"allocation requests. Skipping.\", cn_uuid)"},{"line_number":194,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_39269518","line":191,"range":{"start_line":191,"start_character":38,"end_line":191,"end_character":59},"in_reply_to":"1f1a1f67_4eb0698c","updated":"2017-07-20 14:38:05.000000000","message":"Thanks Matt for noticing that this bug hasn\u0027t been merged yet. Anyhow I already opened a bug report[1] so let\u0027s make sure it is fixed here.\n\n[1] https://launchpad.net/bugs/1705446","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f44bdca36507a92abf6432667cae71056a8c9430","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                spec_obj.instance_group.hosts.append(claimed_host.host)"},{"line_number":222,"context_line":"                # hosts has to be not part of the updates when saving"},{"line_number":223,"context_line":"                spec_obj.instance_group.obj_reset_changes([\u0027hosts\u0027])"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        return selected_hosts"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def _cleanup_allocations(self, instance_uuids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_cabda002","line":224,"updated":"2017-07-19 20:52:44.000000000","message":"nit: unrelated whitespace","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                spec_obj.instance_group.hosts.append(claimed_host.host)"},{"line_number":222,"context_line":"                # hosts has to be not part of the updates when saving"},{"line_number":223,"context_line":"                spec_obj.instance_group.obj_reset_changes([\u0027hosts\u0027])"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        return selected_hosts"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def _cleanup_allocations(self, instance_uuids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_86465821","line":224,"in_reply_to":"1f1a1f67_cabda002","updated":"2017-07-20 23:14:52.000000000","message":"Done","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"65148f2a4631a1a4b5cdcaabbd41baaa45053254","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                spec_obj.instance_group.hosts.append(claimed_host.host)"},{"line_number":222,"context_line":"                # hosts has to be not part of the updates when saving"},{"line_number":223,"context_line":"                spec_obj.instance_group.obj_reset_changes([\u0027hosts\u0027])"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        return selected_hosts"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def _cleanup_allocations(self, instance_uuids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_4a6bd0ef","line":224,"in_reply_to":"1f1a1f67_cabda002","updated":"2017-07-19 21:07:21.000000000","message":"I like it, but I loves me some vertical whitespacing.","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0568a2a88576e0ce8a82db2d5eb2fa12f36c1400","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        for uuid in instance_uuids:"},{"line_number":233,"context_line":"            self.placement_client.delete_allocation_for_instance(uuid)"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def _claim_resources(self, ctx, spec_obj, instance_uuid, alloc_reqs):"},{"line_number":236,"context_line":"        \"\"\"Given an instance UUID (representing the consumer of resources), the"},{"line_number":237,"context_line":"        HostState object for the host that was chosen for the instance, and a"},{"line_number":238,"context_line":"        list of allocation request JSON objects, attempt to claim resources for"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_2212f00a","line":235,"updated":"2017-07-20 07:41:06.000000000","message":"as I said in PS3, I wonder if we would need to use a synchronous section here but given we have a generation bit, two concurrent allocation requests should not be racy, right?","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        for uuid in instance_uuids:"},{"line_number":233,"context_line":"            self.placement_client.delete_allocation_for_instance(uuid)"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def _claim_resources(self, ctx, spec_obj, instance_uuid, alloc_reqs):"},{"line_number":236,"context_line":"        \"\"\"Given an instance UUID (representing the consumer of resources), the"},{"line_number":237,"context_line":"        HostState object for the host that was chosen for the instance, and a"},{"line_number":238,"context_line":"        list of allocation request JSON objects, attempt to claim resources for"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_46696089","line":235,"in_reply_to":"1f1a1f67_2212f00a","updated":"2017-07-20 23:14:52.000000000","message":"right, the generation prevents bad writes and we retry all hosts in the _schedule() method above.","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f44bdca36507a92abf6432667cae71056a8c9430","unresolved":false,"context_lines":[{"line_number":281,"context_line":"        LOG.debug(\"Successfully claimed resources for instance %s using \""},{"line_number":282,"context_line":"                  \"allocation request %s\", instance_uuid, alloc_req)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        return True"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _get_sorted_hosts(self, spec_obj, host_states, index):"},{"line_number":287,"context_line":"        \"\"\"Returns a list of HostState objects that match the required"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_8a07a8ca","line":284,"updated":"2017-07-19 20:52:44.000000000","message":"nit: is there really a need to put a blank line after (nearly) every statement?","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":281,"context_line":"        LOG.debug(\"Successfully claimed resources for instance %s using \""},{"line_number":282,"context_line":"                  \"allocation request %s\", instance_uuid, alloc_req)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        return True"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _get_sorted_hosts(self, spec_obj, host_states, index):"},{"line_number":287,"context_line":"        \"\"\"Returns a list of HostState objects that match the required"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_c612100a","line":284,"in_reply_to":"1f1a1f67_8a07a8ca","updated":"2017-07-20 23:14:52.000000000","message":"sorry, I like vertical whitespace, as you know :)","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"65148f2a4631a1a4b5cdcaabbd41baaa45053254","unresolved":false,"context_lines":[{"line_number":281,"context_line":"        LOG.debug(\"Successfully claimed resources for instance %s using \""},{"line_number":282,"context_line":"                  \"allocation request %s\", instance_uuid, alloc_req)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        return True"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _get_sorted_hosts(self, spec_obj, host_states, index):"},{"line_number":287,"context_line":"        \"\"\"Returns a list of HostState objects that match the required"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_aa526433","line":284,"in_reply_to":"1f1a1f67_8a07a8ca","updated":"2017-07-19 21:07:21.000000000","message":"to me, it makes the diff (and code) so much easier to read","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"84a02771515867a18eed47f1bddd5eeb0edc9e73","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                    LOG.debug(\"Unable to successfully claim against any host.\")"},{"line_number":212,"context_line":"                    self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":213,"context_line":"                    return []"},{"line_number":214,"context_line":"                elif instance_uuids is not None:"},{"line_number":215,"context_line":"                    claimed_instance_uuids.append(instance_uuid)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"            LOG.debug(\"Selected host: %(host)s\", {\u0027host\u0027: claimed_host})"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f1a1f67_d712a109","line":214,"range":{"start_line":214,"start_character":21,"end_line":214,"end_character":35},"updated":"2017-07-21 14:41:50.000000000","message":"Should be \u0027instance_uuid\u0027","commit_id":"0a5618788663394a583645c07ffe37a3dd040f95"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"62c816de47c2e64d939b0ec710b251a3e5078ead","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                    LOG.debug(\"Unable to successfully claim against any host.\")"},{"line_number":212,"context_line":"                    self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":213,"context_line":"                    return []"},{"line_number":214,"context_line":"                elif instance_uuids is not None:"},{"line_number":215,"context_line":"                    claimed_instance_uuids.append(instance_uuid)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"            LOG.debug(\"Selected host: %(host)s\", {\u0027host\u0027: claimed_host})"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f1a1f67_fb1a2715","line":214,"range":{"start_line":214,"start_character":21,"end_line":214,"end_character":35},"in_reply_to":"1f1a1f67_d712a109","updated":"2017-07-21 15:11:36.000000000","message":"No, unfortunately the above is correct :) the instance_uuids parameter is None when old conductors call a newer scheduler endpoint.\n\nHowever, it should not be in an elif and should be de-indented one level, so good spot :)","commit_id":"0a5618788663394a583645c07ffe37a3dd040f95"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1bed784f1ce1795c11fa84522219fb62e062a63b","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        \"\"\"Returns a list of hosts that meet the required specs, ordered by"},{"line_number":114,"context_line":"        their fitness."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        These hosts will have already had their resources claimed in Placement."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"        :param context: The RequestContext object"},{"line_number":119,"context_line":"        :param spec_obj: The RequestSpec object"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_ec21394f","line":116,"range":{"start_line":116,"start_character":21,"end_line":116,"end_character":41},"updated":"2017-07-25 12:54:51.000000000","message":"I\u0027m unclear as a non-english-native speaker whether it\u0027s either past or future tense here but I guess you mean that after this call, hosts will have resources claimed, right?","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7523822040022be249866921ab20f4cc53ee122b","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                # host in the list of sorted hosts. In addition to older"},{"line_number":176,"context_line":"                # conductors, we need to support the caching scheduler, which"},{"line_number":177,"context_line":"                # doesn\u0027t use the placement API (and has"},{"line_number":178,"context_line":"                # USES_ALLOCATION_CANDIDATE \u003d False) and therefore we skip all"},{"line_number":179,"context_line":"                # the claiming logic for that scheduler driver. Finally, if"},{"line_number":180,"context_line":"                # there was a problem communicating with the placement API,"},{"line_number":181,"context_line":"                # alloc_reqs_by_rp_uuid will be None, so we skip claiming in"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_4cd6052a","line":178,"range":{"start_line":178,"start_character":18,"end_line":178,"end_character":43},"updated":"2017-07-25 12:53:14.000000000","message":"micro-nit: this should be plural","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"baf9b4be7eaf2676decbcbdfedd2a9fc1ff6a969","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                claimed_host \u003d None"},{"line_number":192,"context_line":"                for host in hosts:"},{"line_number":193,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":194,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":195,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""},{"line_number":196,"context_line":"                                  \"allocation requests. Skipping.\", cn_uuid)"},{"line_number":197,"context_line":"                        continue"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"                    alloc_reqs \u003d alloc_reqs_by_rp_uuid[cn_uuid]"},{"line_number":200,"context_line":"                    if self._claim_resources(elevated, spec_obj, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_5bf10b6b","line":197,"range":{"start_line":194,"start_character":20,"end_line":197,"end_character":32},"updated":"2017-07-25 21:37:05.000000000","message":"The ironic nv job that failed on this change failed here:\n\nhttp://logs.openstack.org/66/483566/10/check/gate-tempest-dsvm-ironic-ipa-wholedisk-bios-agent_ipmitool-tinyipa-ubuntu-xenial-nv/3a475d1/logs/screen-n-sch.txt.gz#_Jul_24_16_26_25_372045\n\nThe HostState passed all of the filters, but apparently isn\u0027t in the allocation requests returned from Placement.\n\nSo we should probably figure out why.","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1bed784f1ce1795c11fa84522219fb62e062a63b","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                    alloc_reqs \u003d alloc_reqs_by_rp_uuid[cn_uuid]"},{"line_number":200,"context_line":"                    if self._claim_resources(elevated, spec_obj, instance_uuid,"},{"line_number":201,"context_line":"                            alloc_reqs):"},{"line_number":202,"context_line":"                        claimed_host \u003d host"},{"line_number":203,"context_line":"                        break"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"                if claimed_host is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_4ce045cd","line":202,"range":{"start_line":202,"start_character":39,"end_line":202,"end_character":43},"updated":"2017-07-25 12:54:51.000000000","message":"how can we be sure that \u0027host\u0027 is actually corresponding to the first allocation request passed in alloc_reqs ?\nSee, I\u0027d prefer to verify that alloc_reqs[0].key() equals to that one, WDYT ?","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"683d36c0b769eefee2e337689d0e17ae24749119","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                    alloc_reqs \u003d alloc_reqs_by_rp_uuid[cn_uuid]"},{"line_number":200,"context_line":"                    if self._claim_resources(elevated, spec_obj, instance_uuid,"},{"line_number":201,"context_line":"                            alloc_reqs):"},{"line_number":202,"context_line":"                        claimed_host \u003d host"},{"line_number":203,"context_line":"                        break"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"                if claimed_host is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_d2a96edd","line":202,"range":{"start_line":202,"start_character":39,"end_line":202,"end_character":43},"in_reply_to":"ff346bd7_4ce045cd","updated":"2017-07-25 13:28:49.000000000","message":"See line 199","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ecbbe60a175bc172abb55bfb06eb33144765c29","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                    alloc_reqs \u003d alloc_reqs_by_rp_uuid[cn_uuid]"},{"line_number":200,"context_line":"                    if self._claim_resources(elevated, spec_obj, instance_uuid,"},{"line_number":201,"context_line":"                            alloc_reqs):"},{"line_number":202,"context_line":"                        claimed_host \u003d host"},{"line_number":203,"context_line":"                        break"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"                if claimed_host is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_f22472c8","line":202,"range":{"start_line":202,"start_character":39,"end_line":202,"end_character":43},"in_reply_to":"ff346bd7_d2a96edd","updated":"2017-07-25 13:49:02.000000000","message":"My bad.","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7523822040022be249866921ab20f4cc53ee122b","unresolved":false,"context_lines":[{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def _cleanup_allocations(self, instance_uuids):"},{"line_number":230,"context_line":"        \"\"\"Removes allocations for the supplied instance UUIDs.\"\"\""},{"line_number":231,"context_line":"        if not instance_uuids:"},{"line_number":232,"context_line":"            return"},{"line_number":233,"context_line":"        LOG.debug(\"Cleaning up allocations for %s\", instance_uuids)"},{"line_number":234,"context_line":"        for uuid in instance_uuids:"},{"line_number":235,"context_line":"            self.placement_client.delete_allocation_for_instance(uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_8c72ad48","line":232,"range":{"start_line":231,"start_character":8,"end_line":232,"end_character":18},"updated":"2017-07-25 12:53:14.000000000","message":"Technically, the for loop will just return if the list is empty, so this check isn\u0027t needed. But, meh.","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7523822040022be249866921ab20f4cc53ee122b","unresolved":false,"context_lines":[{"line_number":249,"context_line":"                           allocate against (at least) the compute host"},{"line_number":250,"context_line":"                           selected by the _schedule() method. These allocation"},{"line_number":251,"context_line":"                           requests were constructed from a call to the GET"},{"line_number":252,"context_line":"                           /allocation_candidates placement API call.  Each"},{"line_number":253,"context_line":"                           allocation_request satisfies the original request"},{"line_number":254,"context_line":"                           for resources and can be supplied as-is (along with"},{"line_number":255,"context_line":"                           the project and user ID to the placement API\u0027s"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_0c86bddc","line":252,"range":{"start_line":252,"start_character":69,"end_line":252,"end_character":71},"updated":"2017-07-25 12:53:14.000000000","message":"Horrors! A double space!!!","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1bed784f1ce1795c11fa84522219fb62e062a63b","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        # allocation requests in the future based on information in the"},{"line_number":273,"context_line":"        # provider summaries, we\u0027ll just try to claim resources using the first"},{"line_number":274,"context_line":"        # allocation request"},{"line_number":275,"context_line":"        alloc_req \u003d alloc_reqs[0]"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        claimed \u003d self.placement_client.claim_resources(instance_uuid,"},{"line_number":278,"context_line":"            alloc_req, project_id, user_id)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_ccfe7537","line":275,"range":{"start_line":275,"start_character":20,"end_line":275,"end_character":33},"updated":"2017-07-25 12:54:51.000000000","message":"see, for example, say that allocs_reqs[0] is not accepted by filters, it could be a problem, nope ?","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ecbbe60a175bc172abb55bfb06eb33144765c29","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        # allocation requests in the future based on information in the"},{"line_number":273,"context_line":"        # provider summaries, we\u0027ll just try to claim resources using the first"},{"line_number":274,"context_line":"        # allocation request"},{"line_number":275,"context_line":"        alloc_req \u003d alloc_reqs[0]"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        claimed \u003d self.placement_client.claim_resources(instance_uuid,"},{"line_number":278,"context_line":"            alloc_req, project_id, user_id)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff346bd7_323b1aec","line":275,"range":{"start_line":275,"start_character":20,"end_line":275,"end_character":33},"in_reply_to":"ff346bd7_ccfe7537","updated":"2017-07-25 13:49:02.000000000","message":"Nevermind the above, I was confused with the structure, I thought that alloc_reqs was list of allocation requests having different compute nodes, which is not the case at all : all items of that list share the same compute node as the root resource provider.","commit_id":"144bdfc29ea596b138e5006e9ea15b1b14f06db1"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"1a9834942f7710fd6d2fecd85f4491d5c24943a3","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for num in range(num_instances):"},{"line_number":164,"context_line":"            hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":165,"context_line":"            if not hosts:"},{"line_number":166,"context_line":"                break"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            if (instance_uuids is None or"},{"line_number":169,"context_line":"                    not self.USES_ALLOCATION_CANDIDATES or"}],"source_content_type":"text/x-python","patch_set":11,"id":"ff346bd7_9784dfb2","line":166,"updated":"2017-07-26 03:00:57.000000000","message":"if we break at here, that means the prior instance already claimed won\u0027t be cleaned up.","commit_id":"798872da2f4e9f25a08c299098e9b2de65dc69e2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fc4e4d7c9d06e8a351a1469e0f39d6a0169950be","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for num in range(num_instances):"},{"line_number":164,"context_line":"            hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":165,"context_line":"            if not hosts:"},{"line_number":166,"context_line":"                break"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            if (instance_uuids is None or"},{"line_number":169,"context_line":"                    not self.USES_ALLOCATION_CANDIDATES or"}],"source_content_type":"text/x-python","patch_set":11,"id":"ff346bd7_f44c3b9f","line":166,"in_reply_to":"ff346bd7_9784dfb2","updated":"2017-07-26 11:19:28.000000000","message":"Yeah, looks like there wasn\u0027t a unit test for this situation.","commit_id":"798872da2f4e9f25a08c299098e9b2de65dc69e2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fc4e4d7c9d06e8a351a1469e0f39d6a0169950be","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                claimed_host \u003d None"},{"line_number":192,"context_line":"                for host in hosts:"},{"line_number":193,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":194,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":195,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""},{"line_number":196,"context_line":"                                  \"allocation requests. Skipping.\", cn_uuid)"},{"line_number":197,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":11,"id":"ff346bd7_14557f1d","line":194,"updated":"2017-07-26 11:19:28.000000000","message":"Doesn\u0027t appear to be unit test coverage for this situation either. Probably not importat given it is just for logging.","commit_id":"798872da2f4e9f25a08c299098e9b2de65dc69e2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"34d7f09bf688b812f1191e0d29f459e2e91e1869","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                claimed_host \u003d None"},{"line_number":192,"context_line":"                for host in hosts:"},{"line_number":193,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":194,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":195,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""},{"line_number":196,"context_line":"                                  \"allocation requests. Skipping.\", cn_uuid)"},{"line_number":197,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":11,"id":"ff346bd7_b610f0a9","line":194,"in_reply_to":"ff346bd7_14557f1d","updated":"2017-07-26 15:20:48.000000000","message":"Well we did actually have a real bug here with the IronicNodeState:\n\nhttps://review.openstack.org/#/c/487227/","commit_id":"798872da2f4e9f25a08c299098e9b2de65dc69e2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"03ac4ccfdff5fc2fdc0e1b1a72dcc0bc6430abd1","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_17869667","line":170,"updated":"2017-07-26 16:17:05.000000000","message":"what if we called the scheduler because of a move operation like live-migrate ? In that case, we would delete the allocations for the instance, right? But the instance would still work in the source host, right? Don\u0027t you think it could be a problem, unless we self-heal allocations by the source compute host ?","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"88d9c90a9feef556b2d5275d8e0965f15ce6ae71","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_b7110a07","line":170,"in_reply_to":"ff346bd7_17869667","updated":"2017-07-26 16:18:40.000000000","message":"TBC, I know we self-heal allocations thanks to the RT every periodic run, but I do wonder if it would be a race problem.","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4ac027b7e8a5269b4ae4a0597c316515891d8217","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_78c70e66","line":170,"in_reply_to":"ff346bd7_17869667","updated":"2017-07-26 18:47:51.000000000","message":"Yeah, we discussed this situation in one of the hangouts while you were on vacation.\n\nBasically, what will happen in a move situation is that the allocation against the source host will be deleted (either when the claim_resources() call is done below on the destination host or during _cleanup_allocations()) We are OK with that because:\n\nIn the case of the destination host successfully claiming the instance resources, we don\u0027t want the source host to have resources claimed against it for the same instance anyway.\n\nIn the case of a *failed* claim_resources() against the destination host *or* in the case when _cleanup_allocations() is run when *no* destination host is identified, the update_available_resource() interval method on the original source compute node will end up re-PUT\u0027ing the allocations against the source compute host and essentially resetting the state of things to what they were originally.\n\nIn the future, I\u0027d like to clean up the resource tracking around move operations, but as it stands now (and here in this patch), the *eventual* state of the claimed resources will indeed be accurate.","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"518c562e3e5dff71c1a91ff2dc5367bf2b950770","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_41ad4ae7","line":170,"in_reply_to":"ff346bd7_217ffc15","updated":"2017-07-27 17:50:17.000000000","message":"Yup, it took me awhile to realize that yesterday too. :)","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d32324f6575b6ec2c96b16739c59ee54b2b5fee3","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_217ffc15","line":170,"in_reply_to":"ff346bd7_5c330c15","updated":"2017-07-27 07:33:16.000000000","message":"oh..damn it. we delete the original allocation records in the \u0027PUT /allocations\u0027","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6568c8c3185b28e04a74eb3f7aa6675c712d786d","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_ce863014","line":170,"in_reply_to":"ff346bd7_78c70e66","updated":"2017-07-26 19:59:00.000000000","message":"Okay, so Jay and I hashed this over again just now and we\u0027re all a little bit right and a little bit wrong.\n\nSylvain\u0027s underlying concern is correct, although it\u0027s not because of the quoted line. The quoted line here only happens when num_instances\u003e1, which is never the case for a move operation and thus we will never delete instance allocations during this abort procedure.\n\nHowever, what _is_ correct is that if we succeed in this scheduling operation, we will _replace_ the allocation the instance has with one for the source node. For a window, the scheduler will then think that the source host has freed up resources and send things there that cannot fit. Shortly after, the source compute will \"heal\" its allocations, deleting the new allocation for the destination node that we created in this code. That will cause the destination compute to look like it has free resources, but does not. Finally, when the migration completes, things will heal and the allocation on the target host will remain. However, for the (potentially hours-long) period while the migration is going on, the scheduler is sending things all over the place that can never fit, and in a world where we can\u0027t have reschedules, things are bad.\n\nThe solution we discussed is:\n\nIn this code (path) we can tell if we\u0027re doing a move operation by looking in placement for the instance uuid to see if there are existing allocations. If so, we can assume this is a move operation (a reschedule would have already deleted the allocation from the first host). We merge the allocations for the existing host and the new one we selected here and atomically claim those in placement. That locks the resources for both source and destination nodes. When the migration completes (or fails) the healing of the source or destination (wherever the instance remains) will un-claim for its peer.\n\nNote that this means we need to make sure that the sending host\u0027s healing code will not heal over a migrating instance\u0027s double claim.\n\nJay is going to prepend something before this to do that merged double claim in the scheduler path, which will make this code work.","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"56803bad9415fc40d9c6a08d80ab7c12a63e1e62","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_89cf0a55","line":170,"in_reply_to":"ff346bd7_b7110a07","updated":"2017-07-26 16:49:55.000000000","message":"Yeah, I think you\u0027re right. We really need to clean up allocations by instance and resource provider, not just blindly by instance. By the same token, I guess after a successful migration we have a period where the sending compute still has allocations for the instance until the next heal, unless we fix up that path to delete them immediately? Or maybe we already trigger the RT (and thus the heal) when we\u0027re done with that?","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0eb5584b9bbe27ffa89b803986c2c004e51b93e1","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_d096ce4c","line":170,"in_reply_to":"ff346bd7_ce863014","updated":"2017-07-26 20:41:30.000000000","message":"\u003e The quoted line here only happens when num_instances\u003e1, which is never the case for a move operation and thus we will never delete instance allocations during this abort procedure.\n\nWhy couldn\u0027t we get here in a move operation? num_instances\u003d1 and _get_sorted_hosts returns [] because the hosts were all filtered out already. Oh I see, because claimed_instance_uuids would be empty and _cleanup_allocations noops if claimed_instance_uuids is empty.\n\n\u003e However, what _is_ correct is that if we succeed in this scheduling operation, we will _replace_ the allocation the instance has with one for the source node.\n\nWouldn\u0027t we be PUTing an allocation for the destination node? I guess I don\u0027t get the statement.\n\n\u003e In this code (path) we can tell if we\u0027re doing a move operation by looking in placement for the instance uuid to see if there are existing allocations. If so, we can assume this is a move operation (a reschedule would have already deleted the allocation from the first host).\n\nYeah we talked about this a couple of weeks ago but must have forgotten about it.","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f4e92a0eabd7cea12cd7189bd2d364b8a75c0323","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                # in instance_uuids were able to be matched to a selected host."},{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_5c330c15","line":170,"in_reply_to":"ff346bd7_d096ce4c","updated":"2017-07-27 06:43:07.000000000","message":"so the claim at line 205 just appends more allocation records? that means the allocation records never be removed. So that means we are good on the move action..\n\nMaybe I didn\u0027t follow the problem...","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"03ac4ccfdff5fc2fdc0e1b1a72dcc0bc6430abd1","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                    # this request and return an empty list which will cause"},{"line_number":215,"context_line":"                    # select_destinations() to raise NoValidHost"},{"line_number":216,"context_line":"                    LOG.debug(\"Unable to successfully claim against any host.\")"},{"line_number":217,"context_line":"                    self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":218,"context_line":"                    return []"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"                claimed_instance_uuids.append(instance_uuid)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff346bd7_f7d8b27b","line":217,"updated":"2017-07-26 16:17:05.000000000","message":"here, that\u0027s acceptable to cleanup the allocations given _claim_resources() previously already removed the previous allocations for a move operation. I just wonder if we should put again the older allocations if something went wrong here.","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e9a1a76a9b650748b07fb6f431dc6a56d0bad04","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"},{"line_number":174,"context_line":"                    not self.USES_ALLOCATION_CANDIDATES or"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_fd5a9b80","line":171,"range":{"start_line":171,"start_character":16,"end_line":171,"end_character":21},"updated":"2017-07-27 22:07:48.000000000","message":"Do we need to return [] here like on L218?\n\nCouldn\u0027t selected_hosts already have some stuff in it?","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"55555d96288d3f38738b31f51ebe990ea134062f","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                # So, let\u0027s clean up any already-claimed allocations here"},{"line_number":169,"context_line":"                # before breaking and returning"},{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"},{"line_number":174,"context_line":"                    not self.USES_ALLOCATION_CANDIDATES or"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_3dc143e6","line":171,"range":{"start_line":171,"start_character":16,"end_line":171,"end_character":21},"in_reply_to":"ff346bd7_fd5a9b80","updated":"2017-07-27 22:10:54.000000000","message":"I think we\u0027d be OK because of L86 in select_destinations:\n\nif len(selected_hosts) \u003c num_instances:","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e9a1a76a9b650748b07fb6f431dc6a56d0bad04","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":171,"context_line":"                break"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            if (instance_uuids is None or"},{"line_number":174,"context_line":"                    not self.USES_ALLOCATION_CANDIDATES or"},{"line_number":175,"context_line":"                    alloc_reqs_by_rp_uuid is None):"},{"line_number":176,"context_line":"                # Unfortunately, we still need to deal with older conductors"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_5d5d2713","line":173,"range":{"start_line":173,"start_character":16,"end_line":173,"end_character":38},"updated":"2017-07-27 22:07:48.000000000","message":"I\u0027m not sure when instance_uuids would be None?\n\n(later)\n\nOh I guess:\n\n\u003e # Unfortunately, we still need to deal with older conductors\n# that may not be passing in a list of instance_uuids.\n\nWhich was added in I44ebdb3e29db950bf2ad0e6b1dbfdecd1ca03530.\n\nWe should be able to at least remove that condition in Queens.","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"769c74b8143e534656a5c54ea74fe2cbfce7d457","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                for host in hosts:"},{"line_number":198,"context_line":"                    cn_uuid \u003d host.uuid"},{"line_number":199,"context_line":"                    if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":200,"context_line":"                        LOG.debug(\"Found host state %s that wasn\u0027t in \""},{"line_number":201,"context_line":"                                  \"allocation requests. Skipping.\", cn_uuid)"},{"line_number":202,"context_line":"                        continue"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"                    alloc_reqs \u003d alloc_reqs_by_rp_uuid[cn_uuid]"},{"line_number":205,"context_line":"                    if self._claim_resources(elevated, spec_obj, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_40e09acd","line":202,"range":{"start_line":200,"start_character":24,"end_line":202,"end_character":32},"updated":"2017-07-27 22:42:13.000000000","message":"The coverage report tells me we don\u0027t have a unit test for this.","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e9a1a76a9b650748b07fb6f431dc6a56d0bad04","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                        continue"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"                    alloc_reqs \u003d alloc_reqs_by_rp_uuid[cn_uuid]"},{"line_number":205,"context_line":"                    if self._claim_resources(elevated, spec_obj, instance_uuid,"},{"line_number":206,"context_line":"                            alloc_reqs):"},{"line_number":207,"context_line":"                        claimed_host \u003d host"},{"line_number":208,"context_line":"                        break"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"                if claimed_host is None:"},{"line_number":211,"context_line":"                    # We weren\u0027t able to claim resources in the placement API"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_3d7ca3ea","line":208,"range":{"start_line":205,"start_character":20,"end_line":208,"end_character":29},"updated":"2017-07-27 22:07:48.000000000","message":"Seems like if this is False because we\u0027re doing multiple create and we\u0027ve already exhausted the first host due to earlier instances in the request, we should remove this host from the list of hosts so we don\u0027t waste time on it for the other instances in the list of instance_uuids.\n\nCould be a follow up optimization.","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c91c5f10d7acf82cdce1d8fa803759c73b79f76","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                        continue"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"                    alloc_reqs \u003d alloc_reqs_by_rp_uuid[cn_uuid]"},{"line_number":205,"context_line":"                    if self._claim_resources(elevated, spec_obj, instance_uuid,"},{"line_number":206,"context_line":"                            alloc_reqs):"},{"line_number":207,"context_line":"                        claimed_host \u003d host"},{"line_number":208,"context_line":"                        break"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"                if claimed_host is None:"},{"line_number":211,"context_line":"                    # We weren\u0027t able to claim resources in the placement API"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_3df38376","line":208,"range":{"start_line":205,"start_character":20,"end_line":208,"end_character":29},"in_reply_to":"ff346bd7_3d7ca3ea","updated":"2017-07-27 22:09:08.000000000","message":"I guess we get a fresh host list on L164 anyway:\n\nhosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d9dfe0d2c3c19287eaea4ff681e4803ddd2549ea","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        :param ctx: The RequestContext object"},{"line_number":250,"context_line":"        :param spec_obj: The RequestSpec object"},{"line_number":251,"context_line":"        :param instance_uuid: The UUID of the consuming instance"},{"line_number":252,"context_line":"        :param cn_uuid: UUID of the host to allocate against"},{"line_number":253,"context_line":"        :param alloc_reqs: A list of allocation request JSON objects that"},{"line_number":254,"context_line":"                           allocate against (at least) the compute host"},{"line_number":255,"context_line":"                           selected by the _schedule() method. These allocation"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_5d288795","line":252,"range":{"start_line":252,"start_character":8,"end_line":252,"end_character":60},"updated":"2017-07-27 22:15:59.000000000","message":"melwitt pointed out this isn\u0027t actually passed in","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e9a1a76a9b650748b07fb6f431dc6a56d0bad04","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        # the spec object?"},{"line_number":273,"context_line":"        user_id \u003d ctx.user_id"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        # TODO(jaypipes): Loop through all allocation requests instead of just"},{"line_number":276,"context_line":"        # trying the first one. For now, since we\u0027ll likely want to order the"},{"line_number":277,"context_line":"        # allocation requests in the future based on information in the"},{"line_number":278,"context_line":"        # provider summaries, we\u0027ll just try to claim resources using the first"},{"line_number":279,"context_line":"        # allocation request"},{"line_number":280,"context_line":"        alloc_req \u003d alloc_reqs[0]"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        claimed \u003d self.placement_client.claim_resources(instance_uuid,"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_5dbf67a9","line":279,"range":{"start_line":275,"start_character":8,"end_line":279,"end_character":28},"updated":"2017-07-27 22:07:48.000000000","message":"Yeah seems like this is going to be a pretty major problem when doing multiple create boot requests if we keep trying to hammer the same allocation request.\n\nI know we retry if claim_resources hits a 409:\n\nhttps://review.openstack.org/#/c/486170/\n\nBut say we\u0027re creating 10 instances in the same request and we have two nodes, and there are already 5 instances from another tenant on the first node. Couldn\u0027t we end up filling up the first node and then not even trying the other allocation request for the other node?\n\nDuh, I guess that\u0027s why we loop over hosts on L197. :)","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"769c74b8143e534656a5c54ea74fe2cbfce7d457","unresolved":false,"context_lines":[{"line_number":283,"context_line":"            alloc_req, project_id, user_id)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        if not claimed:"},{"line_number":286,"context_line":"            return False"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        LOG.debug(\"Successfully claimed resources for instance %s using \""},{"line_number":289,"context_line":"                  \"allocation request %s\", instance_uuid, alloc_req)"}],"source_content_type":"text/x-python","patch_set":20,"id":"ff346bd7_60d4bee9","line":286,"range":{"start_line":286,"start_character":12,"end_line":286,"end_character":24},"updated":"2017-07-27 22:42:13.000000000","message":"Coverage report says we need a unit test for this.","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"}],"nova/tests/unit/scheduler/test_filter_scheduler.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f44bdca36507a92abf6432667cae71056a8c9430","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        all_host_states \u003d [host_state]"},{"line_number":74,"context_line":"        mock_get_all_states.return_value \u003d all_host_states"},{"line_number":75,"context_line":"        mock_get_hosts.return_value \u003d all_host_states"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        instance_uuids \u003d None"},{"line_number":78,"context_line":"        ctx \u003d mock.Mock()"},{"line_number":79,"context_line":"        selected_hosts \u003d self.driver._schedule(ctx, spec_obj,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_2aac149d","line":76,"updated":"2017-07-19 20:52:44.000000000","message":"you loves you lotsa blank lines, eh?","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5165afed1ef9d048c115a364d992a3c4b35ee0ca","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        all_host_states \u003d [host_state]"},{"line_number":74,"context_line":"        mock_get_all_states.return_value \u003d all_host_states"},{"line_number":75,"context_line":"        mock_get_hosts.return_value \u003d all_host_states"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        instance_uuids \u003d None"},{"line_number":78,"context_line":"        ctx \u003d mock.Mock()"},{"line_number":79,"context_line":"        selected_hosts \u003d self.driver._schedule(ctx, spec_obj,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f1a1f67_6601fcaf","line":76,"in_reply_to":"1f1a1f67_2aac149d","updated":"2017-07-20 23:14:52.000000000","message":"yes, yes I do.","commit_id":"71a1a9666977e732ff093bf5ca91eed6c8cdd2aa"}],"releasenotes/notes/placement-claims-844540aa7bf52b33.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b577e2f364647b88982ac7eda050d40ad65d9413","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"other:"},{"line_number":3,"context_line":"  - | The filter scheduler will now attempt to claim a number of"},{"line_number":4,"context_line":"    resources in the placement API after determining a list of"},{"line_number":5,"context_line":"    potential hosts. We attempt to claim these resources for each instance"},{"line_number":6,"context_line":"    in the build request, and if a claim does not succeed, we try this"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"1f1a1f67_803abbca","line":3,"range":{"start_line":3,"start_character":6,"end_line":3,"end_character":64},"updated":"2017-07-21 01:21:20.000000000","message":"You need to drop this down a line so it starts under the pipe:\n\nhttp://logs.openstack.org/66/483566/6/check/gate-nova-releasenotes/86e6404/console.html#_2017-07-20_23_30_32_273548","commit_id":"7ba52dad4230df9a221af0be645f31effe9134e3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6790b7b3ec9417f7b08921711cb2d0a62ce6019d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"other:"},{"line_number":3,"context_line":"  - | The filter scheduler will now attempt to claim a number of"},{"line_number":4,"context_line":"    resources in the placement API after determining a list of"},{"line_number":5,"context_line":"    potential hosts. We attempt to claim these resources for each instance"},{"line_number":6,"context_line":"    in the build request, and if a claim does not succeed, we try this"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"1f1a1f67_b9396ef3","line":3,"range":{"start_line":3,"start_character":6,"end_line":3,"end_character":64},"in_reply_to":"1f1a1f67_803abbca","updated":"2017-07-21 03:59:33.000000000","message":"Done","commit_id":"7ba52dad4230df9a221af0be645f31effe9134e3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"03ac4ccfdff5fc2fdc0e1b1a72dcc0bc6430abd1","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"other:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The filter scheduler will now attempt to claim a number of"},{"line_number":5,"context_line":"    resources in the placement API after determining a list of"}],"source_content_type":"text/x-yaml","patch_set":14,"id":"ff346bd7_9742863d","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":5},"updated":"2017-07-26 16:17:05.000000000","message":"given how important scheduler-claims is, a specific \u0027feature\u0027 note looks good to me but meh.","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"03ac4ccfdff5fc2fdc0e1b1a72dcc0bc6430abd1","unresolved":false,"context_lines":[{"line_number":11,"context_line":"    claims against. Claims for resources may fail due to another scheduler"},{"line_number":12,"context_line":"    process concurrently claiming resources against the same compute node."},{"line_number":13,"context_line":"    This concurrent resource claim is normal and the retry of a claim"},{"line_number":14,"context_line":"    request should be unusual but harmless."}],"source_content_type":"text/x-yaml","patch_set":14,"id":"ff346bd7_172a167e","line":14,"updated":"2017-07-26 16:17:05.000000000","message":"I\u0027ll honestly write a prelude section before we version the Pike release so those notes will be summarized at the top of the release document for Pike but thanks for describing those anyway.","commit_id":"c0138c14a0c4e3f97fedd6ee894695a0c121f560"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"700c40d023c6be0fd9200262cbe5b792ec717d2c","unresolved":false,"context_lines":[{"line_number":11,"context_line":"    claims against. Claims for resources may fail due to another scheduler"},{"line_number":12,"context_line":"    process concurrently claiming resources against the same compute node."},{"line_number":13,"context_line":"    This concurrent resource claim is normal and the retry of a claim"},{"line_number":14,"context_line":"    request should be unusual but harmless."}],"source_content_type":"text/x-yaml","patch_set":20,"id":"ff346bd7_e058ae31","line":14,"range":{"start_line":14,"start_character":22,"end_line":14,"end_character":29},"updated":"2017-07-27 22:20:06.000000000","message":"This doesn\u0027t mix very well with \"This concurrent resource claim is normal\" but meh.","commit_id":"d85a50ec6bf4240259285512695b1b9a47c0d6a6"}]}