| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-24 | |||
| 19:04:31 | jaypipes | edleafe: yes, that's exactly correct. I was only pointing out that technically the allocation_requests contain all the information in the alternate_hosts list. | |
| 19:04:48 | dansmith | conductor would throw the first allocation candidate at placement, and then parse the result to determine which compute host it should send the rpc message to | |
| 19:05:04 | jaypipes | edleafe: but like I said, that would require the cell conductor to understand what an allocation request was (i.e. the allocation_request would no longer be opaque) | |
| 19:05:05 | dansmith | that would mean no extra list, but also opaque allocation request | |
| 19:05:13 | dansmith | jaypipes: not if ^ | |
| 19:05:33 | jaypipes | right. :) | |
| 19:05:51 | mriedem | "conductor would throw the first allocation candidate at placement" - that's the cell conductor yes? | |
| 19:05:53 | edleafe | dansmith: "parse the result"? | |
| 19:05:55 | mriedem | during a retry | |
| 19:06:18 | dansmith | edleafe: parse the result of the POST of the allocation | |
| 19:06:37 | jaypipes | I think the most appropriate return value from select_destinations() would be a list of (host, allocation_request) tuples. | |
| 19:07:07 | jaypipes | for the first item in that list, the allocation_request would be the one that had already been successfully claimed for the selected host. | |
| 19:07:21 | jaypipes | dansmith: agree? | |
| 19:07:43 | mriedem | select_destinations today returns as the first entry the one that was chosen, right? | |
| 19:07:45 | dansmith | sure that's fine, if that's how you want it to look | |
| 19:07:54 | jaypipes | mriedem: for each instance in num_instances, yes | |
| 19:08:05 | edleafe | dansmith: reportclient.claim_resources returns a boolean | |
| 19:08:11 | jaypipes | so actually, the return value needs to be list of list of that tuple. | |
| 19:08:27 | dansmith | edleafe: what's your point? | |
| 19:08:36 | jaypipes | with the outer list being for the num_instances | |
| 19:08:37 | edleafe | dansmith: what's there to parse? | |
| 19:08:56 | mriedem | a list of lists of tuples | |
| 19:08:57 | mriedem | what could go wrong | |
| 19:09:12 | edleafe | dansmith: the cell conductor would still need to "know" about the allocation_candidate structure | |
| 19:09:14 | dansmith | edleafe: the actual POST call for /allocations returns the allocation you made right? | |
| 19:09:30 | mriedem | it's not a POST | |
| 19:09:55 | edleafe | It's a PUT | |
| 19:09:58 | dansmith | edleafe: the cell conductor can, but I think it should look at the result of the http call not the thing it was passed in rpc, otherwise we've got version mess | |
| 19:10:09 | edleafe | And it returns a 204 on success | |
| 19:10:14 | dansmith | christ, whatever | |
| 19:10:15 | mriedem | yeah no content on success | |
| 19:10:32 | jaypipes | mriedem: well, tell me if you want to stop supporting num_instances > 1 and I'll gladly submit that patch ;) | |
| 19:10:40 | dansmith | okay then that clearly won't work | |
| 19:11:34 | edleafe | For each host, you get a list of (host, alloc) tuples. | |
| 19:11:50 | jaypipes | right | |
| 19:11:52 | jaypipes | ++ | |
| 19:12:00 | edleafe | On a retry in the cell, you try claiming the alloc. If that succeeds, you build on that host | |
| 19:12:09 | jaypipes | +1 | |
| 19:12:13 | dansmith | I don't love it, but I'm also not sure why we're even discussing it | |
| 19:12:17 | edleafe | If it fails, move to the next one in the list | |
| 19:12:23 | jaypipes | right, zactly.\ | |
| 19:12:55 | edleafe | jaypipes: so I don't understand why you would want to only return alloc | |
| 19:14:14 | mriedem | and just to confirm my understand, we only ever care about the list of lists for the server create case, b/c for everything else, like migrations and unshelve, it gets back the list of host states today but just takes the first one for the rpc cast to compute | |
| 19:14:18 | mriedem | *understanding | |
| 19:14:53 | jaypipes | edleafe: never mind my thought about only returning the allocations. I've been convinced that's a bad idea. | |
| 19:15:22 | edleafe | jaypipes: roger that | |
| 19:15:55 | mriedem | seems you have to have both the HostState and allocation requests because of all the filter properties and az and limits and node crap that's embedded in the HostState object | |
| 19:16:00 | mriedem | which conductor is using before casting to compute | |
| 19:16:02 | mriedem | yeah? | |
| 19:16:19 | dansmith | hope not since hoststate is very scheduler-specific | |
| 19:16:30 | dansmith | all conductor needs is the hostname of the target compute | |
| 19:16:35 | mriedem | sec | |
| 19:16:53 | mriedem | https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L689-L700 | |
| 19:16:57 | mriedem | ^ just for unshelve | |
| 19:17:13 | dansmith | oh, that's not HostState, | |
| 19:17:16 | dansmith | that's the dict of randomness | |
| 19:17:17 | mriedem | but there is all sorts of redonkulous in there for limits and such | |
| 19:17:22 | dansmith | which was based on HostState | |
| 19:17:41 | mriedem | this? https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L48 | |
| 19:17:44 | dansmith | yeah, that's going to fsck us | |
| 19:17:50 | mriedem | still has limits in it | |
| 19:17:54 | mriedem | b/c N-mfing-UMA | |
| 19:17:57 | dansmith | oh well, nice knowing you gents | |
| 19:18:31 | mriedem | well now that dan is taken care of | |
| 19:19:13 | mriedem | and i guess it's getting the az from the chosen host via the aggregates on that host | |
| 19:19:22 | mriedem | limits go into filter properties for the claim in the compute | |
| 19:19:45 | mriedem | and i guess the node is also passed down explicitly for the RT claim | |
| 19:20:45 | mriedem | i wonder if anyone has ever tried shelve offloading and unshelving an instance that had pci/numa stuff on it :) | |
| 19:22:13 | dansmith | that is also an unversioned dict result from that scheduler rpc call | |
| 19:22:15 | dansmith | we should *not* commit that same sin in the boot calls | |
| 19:23:18 | jaypipes | ok, let's focus on what we need to do here to get retries working in cellsv2 | |
| 19:23:35 | jaypipes | are we all in agreement about the proposed return value from select_destinations() for edleafe's patch? | |
| 19:23:43 | dansmith | no I think that's mriedem's point | |
| 19:23:51 | jaypipes | a list of lists of (host, alloc_request) tuples, yes? | |
| 19:23:58 | mriedem | we agree we need allocation requests and the host_state dict thingies i think | |
| 19:24:09 | mriedem | or maybe we don't agree | |
| 19:24:13 | dansmith | I think he's saying we need to pass the HostState mess, right mriedem ? | |
| 19:24:16 | mriedem | yes | |
| 19:24:25 | jaypipes | is even used.... | |
| 19:24:28 | mriedem | it is | |
| 19:24:29 | mriedem | in the claim code | |
| 19:24:32 | dansmith | oh it is | |
| 19:24:33 | dansmith | yeah | |
| 19:24:43 | mriedem | note that all of the docstrings say the limits are vcpus/ram/disk | |
| 19:24:45 | mriedem | not numa | |
| 19:24:48 | mriedem | so it's totally f'ing confusing | |
| 19:25:07 | mriedem | no desert eagle? | |
| 19:25:14 | mriedem | you want open casket? | |
| 19:25:15 | jaypipes | yeah, it is. :( | |
| 19:25:23 | dansmith | mriedem: it's what I have within reach | |
| 19:25:53 | jaypipes | dansmith, mriedem: k, so the returned value needs to be list of lists of (host_dict_with_limits_thing, alloc_request) | |
| 19:25:57 | jaypipes | edleafe: ^ | |
| 19:26:13 | dansmith | no | |
| 19:26:14 | dansmith | because | |
| 19:26:26 | dansmith | host_dict_with_limits is an unversioned structure and we're not adding a parameter with a LIST OF TUPLES OF THAT THING to one of our clean rpc calls | |
| 19:26:51 | mriedem | clean rpc calls? | |
| 19:27:03 | jaypipes | dansmith: don't we already pass the limits stuff down to build_instance()? | |
| 19:27:13 | mriedem | select_destinations today returns the unversioned host_dict_with_limits_thing | |
| 19:27:17 | dansmith | jaypipes: no we get it fromthe scheduler | |
| 19:27:25 | mriedem | jaypipes: the NUMATopologyFilter | |