Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-12
11:37:01 sean-k-mooney i though stephen was filtering the allocation candiate but he is not
11:37:10 efried just question of whether you want fewer LOC I guess
11:37:23 efried stephenfin: https://review.opendev.org/#/c/681719/1/nova/scheduler/manager.py@197
11:37:30 sean-k-mooney update is fine for the summaries
11:37:51 sean-k-mooney i would prefer if we filtered this however
11:37:53 sean-k-mooney Draft
11:38:17 sean-k-mooney alloc_reqs.extend(alloc_reqs_fallback)
11:38:45 efried sean-k-mooney: filter it how?
11:38:59 stephenfin efried: tomato tomato. As you say, it doesn't matter
11:39:07 stephenfin I'm happy to go with '.update()'
11:39:10 sean-k-mooney is that the set of alloction candiates
11:39:13 efried we explicitly can't skip the ones we already have PCPUs for
11:39:20 efried that's the whole point
11:39:35 efried ... I think
11:39:42 sean-k-mooney it would be better not to pass 2 allcoation candiate for any given host
11:39:49 efried because the filter might kick out the PCPU ones
11:40:18 sean-k-mooney efried: we need to make sure if the filter passes we use the PCPU allocation candiate not the VCPU one
11:40:28 sean-k-mooney if we have two we need extra code to handel that
11:40:37 efried That's already covered I thought
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

Earlier   Later