Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-12
11:29:58 luyao sean-k-mooney: Hi, if you get time, could you help look vpmem xml again? thanks ( we discussed before about the numa requiring and alignment, and got a consensus about them)
11:30:29 luyao https://review.opendev.org/#/c/678455/
11:31:51 sean-k-mooney efried: also commented. we should not use update as i noted inline
11:32:22 stephenfin efried: I shouldn't have pushed those mox patches through last night either. Sorry :(
11:32:42 efried no biggie stephenfin
11:33:00 efried they're all going to get kicked out here in a minute
11:33:05 sean-k-mooney luyao: yep i have it open efried pinged me earlier
11:33:29 sean-k-mooney im just going to grab coffe then ill review the vpmem seriese in full starting with the xml patch
11:33:31 stephenfin cool. I switched to -2 on the bottom one to make it clear
11:33:36 luyao sean-k-mooney: hah, thanks
11:35:00 stephenfin On the other hand, once that lands mox is basically gone. It's only been four years \o/
11:35:19 efried sean-k-mooney, stephenfin: It's not important enough to spend a bunch of time on, but am I crazy about .update()?
11:35:38 stephenfin in relation to...?
11:35:42 efried a psum is a psum
11:36:16 efried the way you're doing it I guess you're not replacing one that's already there, whereas .update would (redundantly) replace
11:36:33 stephenfin I need a link
11:36:45 efried I guess python probably isn't smart enough to skip
11:36:48 sean-k-mooney oh maybe i miss understood
11:36:52 efried so yeah, I guess the way you're doing it is going to be more efficient
11:36:56 efried but the result will be the same
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

Earlier   Later