| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-12 | |||
| 11:40:37 | efried | That's already covered I thought | |
| 11:40:43 | sean-k-mooney | where | |
| 11:40:43 | efried | because the candidates are in order per host | |
| 11:41:06 | efried | .extend is O(1) (I think) so we're just taking a mem hit by not filtering. Whereas if we loop through and try to filter, we're O(N). | |
| 11:41:07 | sean-k-mooney | if they are ordered and we take the first one then ok | |
| 11:41:11 | efried | y | |
| 11:41:24 | efried | we noodled this out with gibi earlier I think | |
| 11:41:27 | stephenfin | To do that, we'd need to inspect each entry in alloc_reqs_fallback, pull out the resource provider ID, and check if it exists in alloc_reqs | |
| 11:41:34 | stephenfin | and I think they're ordered so it won't matter | |
| 11:41:53 | sean-k-mooney | efried: exthend is fine if we have odering and can rely on it | |
| 11:42:11 | sean-k-mooney | i just was not aware that was a thing | |
| 11:42:16 | stephenfin | I could do this (I think) | |
| 11:42:35 | efried | stephenfin: I'd rather leave it I think | |
| 11:42:47 | sean-k-mooney | ya i think its fine | |
| 11:42:51 | efried | unless some good reason to do otherwise | |
| 11:42:52 | stephenfin | alloc_reqs.extend([a for a in alloc_reqs_fallback if a['allocations'][path to rp UUID] not in provider_summaries) | |
| 11:43:17 | efried | still O(N) | |
| 11:43:20 | sean-k-mooney | im just trying to make sure we dont pick the wrong allcoatin candiage and end up using VCPUs when we have pinned cores on a host that has both | |
| 11:43:22 | stephenfin | Nah, it's simpler as-is and avoids us doing extra work on what could be a very big list | |
| 11:43:23 | stephenfin | Yup | |
| 11:43:41 | efried | ++ | |
| 11:43:41 | stephenfin | sean-k-mooney: Remember, the NUMATopologyFilter will prevent that | |
| 11:43:58 | sean-k-mooney | stephenfin: no the numa toplogy filter will only pass or fail a host | |
| 11:44:08 | sean-k-mooney | stephenfin: it has no say over the allocation candiates | |
| 11:44:18 | sean-k-mooney | so it will pass or fail both | |
| 11:44:35 | sean-k-mooney | filters today work on host not allocation candiates | |
| 11:44:37 | stephenfin | ah, right | |
| 11:44:47 | sean-k-mooney | that is why the ordering is important | |
| 11:44:49 | stephenfin | so we'd be using the VCPU-based allocation candidate blob | |
| 11:44:52 | sean-k-mooney | and that we take the first one | |
| 11:45:25 | stephenfin | but the host has both 'NUMATopology.cpuset' and 'NUMATopology.pcpuset' set to something | |
| 11:45:44 | stephenfin | and the NUMATopologyFilter would use the latter and pass the request | |
| 11:45:55 | sean-k-mooney | yes | |
| 11:46:08 | stephenfin | Okay, I'll triple check that ordering is preserved | |
| 11:46:26 | sean-k-mooney | its an issue only if it we have free inventory of both VCPUs and PCPUs | |
| 11:46:37 | sean-k-mooney | and have allocation candate for both | |
| 11:46:49 | stephenfin | agreed | |
| 11:46:51 | sean-k-mooney | in which case we need to make sure we use the PCPU inventory | |
| 11:46:57 | sean-k-mooney | *allocation_candiate | |
| 11:47:02 | stephenfin | a corner case, but there's no reason to give someone that loaded gun | |
| 11:47:38 | sean-k-mooney | i prefer not giving people firearms at all | |
| 11:48:06 | donnyd | sean-k-mooney: something is wrong with the mirror at FN | |
| 11:48:08 | sean-k-mooney | sowrd at noon is much more civalised then pistols at dawn | |
| 11:48:38 | donnyd | I am working on getting it fixed | |
| 11:48:55 | stephenfin | https://www.youtube.com/watch?v=vtnS3_bmtm8 | |
| 11:49:05 | sean-k-mooney | hurleys are a perfectly suitable alternitive | |
| 11:49:36 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow resizing server with port resource request https://review.opendev.org/679019 | |
| 11:49:36 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not query allocations twice in finish_revert_resize https://review.opendev.org/678827 | |
| 11:51:13 | sean-k-mooney | stephenfin: i havent seen that in a while | |
| 11:54:37 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Extract pf$N literals as constants from func test https://review.opendev.org/680991 | |
| 11:57:05 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Improve dest service level func tests https://review.opendev.org/680998 | |
| 11:57:06 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Follow up for Ib50b6b02208f5bd2972de8a6f8f685c19745514c https://review.opendev.org/681490 | |
| 11:57:15 | alex_xu | stephenfin: for sean-k-mooney pointed case both have VCPU and PCPU, then we only keep the PCPU allocation_candidtes override VCPu allocation_candidtes in alloc_reqs, then it will be ok. | |
| 11:57:51 | alex_xu | if both PCPU and VCPU on the same host, that VCPU must be for shared cpu | |
| 11:58:22 | sean-k-mooney | right | |
| 11:58:57 | sean-k-mooney | but i did not see code to do that in stephens code | |
| 11:59:08 | sean-k-mooney | that siad i only quickly looked at it | |
| 11:59:27 | alex_xu | yea, nice spot | |
| 11:59:35 | stephenfin | alex_xu: Yeah, we don't have that. Instead we're relying on ordering preventing that from happening | |
| 11:59:43 | stephenfin | I think I should add the filtering, to be safe | |
| 12:00:11 | sean-k-mooney | if we were building a dict of host to allocation candiate we coudl use set_default | |
| 12:00:14 | stephenfin | Scenario: we only have one host that provides both PCPU and VCPU | |
| 12:01:05 | stephenfin | actually, I was going to say the the NUMATopology filter fails the request with PCPUs but passes the VCPU request | |
| 12:01:07 | stephenfin | would that happen? | |
| 12:01:11 | stephenfin | ...though? | |
| 12:01:21 | sean-k-mooney | no | |
| 12:01:29 | stephenfin | I guess something could have changed in the time between the two tests | |
| 12:01:30 | sean-k-mooney | the filter does not operate on requests | |
| 12:01:34 | sean-k-mooney | they operate on hosts | |
| 12:01:50 | stephenfin | yeah, correct | |
| 12:02:16 | stephenfin | so if our order is preserved, we'll have hit that host already due to the allocation request for PCPUs | |
| 12:02:41 | stephenfin | if it failed once, it will fail if we hit it a second time due to the allocation request with VCPUs | |
| 12:02:43 | stephenfin | right? | |
| 12:03:00 | alex_xu | no, the scheduler filtered the hosts by allocation-candidtes first, then iterate the filtered hosts | |
| 12:03:03 | efried | "something could have changed" disregard this, already a window between GET /a_c and PUT /allocs | |
| 12:04:03 | stephenfin | efried: I mean we take the allocation request with PCPUs and try passing that through the NUMATopologyFilter or something and it fails | |
| 12:04:05 | sean-k-mooney | i though we got all the host form all allcoation candiate and looped over them once | |
| 12:04:14 | stephenfin | we then try a load of other hosts, which all fail | |
| 12:04:15 | sean-k-mooney | applying all fiters | |
| 12:04:28 | sean-k-mooney | then passed that to the weighers | |
| 12:04:34 | stephenfin | then we come back to that same original host only using the allocation candidate with VCPUs | |
| 12:04:42 | sean-k-mooney | and finally select a host then find the corresponing allcoaiton candiate | |
| 12:04:43 | alex_xu | stephenfin: maybe we ignore the allocation_request if the rp_uuid already in alloc_reqs_by_rp_uuid https://review.opendev.org/#/c/681719/1/nova/scheduler/manager.py@212 | |
| 12:05:06 | stephenfin | only in the time inbetween, something has changed on the host so the NUMATopologyFilter or whatever passes the request | |
| 12:05:24 | stephenfin | it's going to be such a tiny window but I wonder if it's worth worrying about? | |
| 12:05:47 | sean-k-mooney | stephenfin: if something changes the claim wil fial in placmeent | |
| 12:05:48 | alex_xu | ah, no, I'm wrong, we can't ignore directly, since we have same rp for child rp | |
| 12:05:53 | sean-k-mooney | then we will move on to the next host | |
| 12:06:09 | efried | I wouldn't have thought it was any different than what we're doing now for multiple candidates on one host. | |
| 12:06:27 | stephenfin | no, it's not really actually | |
| 12:06:38 | efried | sean-k-mooney: I think he's talking about if the second one spuriously succeeds | |
| 12:07:01 | efried | but yeah, I don't think this is worth worrying about. | |
| 12:07:05 | sean-k-mooney | my point is i dont think the numa toplogy filter will ever run twice for the same host | |
| 12:07:11 | sean-k-mooney | it will only run once | |
| 12:07:18 | efried | and i think that ^ is what we want | |
| 12:07:19 | alex_xu | ^ agree | |
| 12:07:32 | alex_xu | agree to sean-k-mooney... :) | |
| 12:07:33 | sean-k-mooney | because filter today have no knowadge of allcoation candates | |
| 12:08:14 | efried | because if there are available PCPUs for a host, we want to check that candidate only. But if there aren't any, there will only be (at most) VCPU candidates in the list. | |
| 12:08:47 | sean-k-mooney | if there are free pcpus and vcpus we will pass the host and have two candiates | |