Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-12
11:40:43 efried because the candidates are in order per host
11:40:43 sean-k-mooney where
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 stephenfin sean-k-mooney: Remember, the NUMATopologyFilter will prevent that
11:43:41 efried ++
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: Do not query allocations twice in finish_revert_resize https://review.opendev.org/678827
11:49:36 openstackgerrit Balazs Gibizer proposed openstack/nova master: Allow resizing server with port resource request https://review.opendev.org/679019
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
12:09:00 sean-k-mooney and hopefully rely on orderr to pic the PCPU one

Earlier   Later