| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-12 | |||
| 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 | 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 | |