| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-11 | |||
| 14:24:19 | bauzas | stephenfin: ^ | |
| 14:24:41 | bauzas | if we change the capacity for VGPU, then it could be a problem | |
| 14:24:56 | dansmith | stephenfin: another thing I'd support is converting instances to allocations that are maybe overly conservative.. like if you need to reserve more resources than they really have to make the math work out, that seems like a potential compromise | |
| 14:24:59 | bauzas | lass for instances that are taking VCPUs located on CPU2 to 7 and allocate PCPU for instances that are taking CPU0 and 1. But if ratio is 16, we could have 32 instances (asking for 1 VCPU) to be allocated against 2 PCPU with ratio=1.0." | |
| 14:24:59 | bauzas | "I'll try to clarify my thoughts with an upgrade example on a host with 8 physical CPUs, named CPU0 to 7:in Stein, instances are running and actively taking VCPUs that are consuming CPU0 to CPU7.in Train, operator wants to dedicate CPU0 and CPU1. Accordingly, CPU2 to 7 will be shared.Consequently, VCPU inventory for that host will be reduced by the amount of allocation_ratio * 2. In theory, we should then allocate VCPU resource | |
| 14:25:09 | dansmith | stephenfin: and then recalculate that on migrate if they want.. depending on how that looks | |
| 14:25:32 | dansmith | I dunno what the actual complexity concern looks like, so I'm just spitballing | |
| 14:25:38 | stephenfin | dansmith: That might be necessary for something like 'hw:cpu_threads_policy=isolate' | |
| 14:25:43 | dansmith | yeah | |
| 14:27:03 | stephenfin | bauzas: Yeah, I think we need a startup check to ensure NUM_VCPUS_USED_BY_INSTANCE <= NUM_VCPUS_AVAILABLE | |
| 14:27:08 | stephenfin | *INSTANCES | |
| 14:27:15 | sean-k-mooney | well we say in the spec hw:cpu_threads_policy woudl be going away | |
| 14:27:42 | bauzas | stephenfin: cool with it then | |
| 14:27:55 | bauzas | stephenfin: but then we need to call placement when restarting the compute service | |
| 14:28:01 | stephenfin | sean-k-mooney: Yup, but there has to be an intermediate step that lets us account for the fact that existing instances are using more cores than instance.vcpus | |
| 14:28:07 | bauzas | *every time* | |
| 14:28:29 | bauzas | for vGPUs, we basically only reshape once | |
| 14:28:45 | stephenfin | bauzas: Hmm, could also be a nova-status check as dansmith suggested | |
| 14:28:46 | mnaser | what's the concern in taking the current state and translating that directly into placement when the compute node goes up? | |
| 14:28:58 | dansmith | mnaser: complexity | |
| 14:29:04 | dansmith | but we have to bite that bullet I think | |
| 14:29:15 | stephenfin | mnaser: there are a lot of ways things can be inconsistent and we need to handle those | |
| 14:29:22 | sean-k-mooney | stephenfin: that only happens for pinned instacnes if the hsot has hyper treading or you have emulartor_treads=isolate | |
| 14:29:39 | sean-k-mooney | stephenfin: but yes we do | |
| 14:29:44 | stephenfin | like the way there's nothing preventing you from scheduling pinned and unpinned instances on the same host | |
| 14:30:25 | mnaser | couldn't you introspect pinned and unpinned from the libvirt definition | |
| 14:30:41 | sean-k-mooney | mnaser: we can tell form the flavor | |
| 14:30:42 | stephenfin | (so when we migrate, we could end up in a situation where an N core host could have N PCPUs and N * overallocation_ratio VCPUs in use at the same time) | |
| 14:31:14 | sean-k-mooney | its not a case of we dont know this happens we told operators that its there respociblity to ensure it does not | |
| 14:31:35 | sean-k-mooney | that is the issue we told them to do somthing but did not enforec it in code | |
| 14:31:48 | sean-k-mooney | there for we have to assuem the worst | |
| 14:32:01 | stephenfin | or the fact that when using the isolate cpu thread policy, the instance may or may not be using twice as many cores as its supposed to be using (isolate will reserve the hyperthread siblings for each core used by the instance) | |
| 14:32:25 | stephenfin | sean-k-mooney: Correct | |
| 14:32:33 | sean-k-mooney | yes alther to be faire we do account for that properly in the resocue tracker | |
| 14:32:44 | sean-k-mooney | *although | |
| 14:32:50 | stephenfin | yup | |
| 14:33:13 | sean-k-mooney | mnaser: so we have all the data to fix things if we need to | |
| 14:33:26 | bauzas | stephenfin: we *could* do it with nova-status but then operators would have to migrate (or delete some instances) :( | |
| 14:33:32 | sean-k-mooney | the issue is that the isolate pollicy is not compatible with placcement | |
| 14:33:40 | bauzas | thanks, allocation ratio | |
| 14:33:43 | mriedem | lyarwood: can you hit these to keep things rolling https://review.openstack.org/#/q/topic:bug/1669054+branch:stable/rocky | |
| 14:33:50 | sean-k-mooney | it chagnes the quantity fo resocue based in the hsot that is selected | |
| 14:34:25 | sean-k-mooney | bauzas: no i think we can fix allocation for existing instance | |
| 14:34:46 | bauzas | sean-k-mooney: how ? see my example | |
| 14:34:54 | sean-k-mooney | the thing we have to be ok with is removing cpu_thread policies | |
| 14:35:12 | sean-k-mooney | bauzas: we can over allocate RPs if we need to initally | |
| 14:36:29 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: libvirt: disconnect volume when encryption fails https://review.openstack.org/651796 | |
| 14:36:46 | sean-k-mooney | or we can say you asked for 2 cpus but you have isolate and are artully using 4 cpus and update the placement allcoation accordingly | |
| 14:37:00 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Don't warn on network-vif-unplugged event during live migration https://review.openstack.org/651797 | |
| 14:37:34 | stephenfin | mriedem: If you're looking at stable stuff, think you could look at these too? https://review.openstack.org/#/c/650363/ https://review.openstack.org/#/c/650364/ | |
| 14:37:39 | sean-k-mooney | with pinned cpus there was no over subscirtion so the fact the vm is there means it can fit and we correctly do the accounting in the resouce tracker to hanel the addtional cpu usage | |
| 14:38:28 | mriedem | stephenfin: ok | |
| 14:38:33 | stephenfin | thanks | |
| 14:38:42 | stephenfin | bauzas: You would, but is there anyway to work around that? | |
| 14:39:01 | stephenfin | I mean, if they're in a broken state, something has to change | |
| 14:39:21 | stephenfin | bauzas: Also, wouldn't this exact same thing happen now if you messed with allocation ratios? | |
| 14:40:07 | stephenfin | i.e. If there are already instances on a host and I drop cpu_allocation_ratio from 16.0 to 2.0 and restart nova-compute, what happens? | |
| 14:40:11 | bauzas | well, I dunno what to say | |
| 14:40:20 | bauzas | stephenfin: it just works | |
| 14:40:30 | sean-k-mooney | in the placement side the ratio changes | |
| 14:40:37 | bauzas | stephenfin: but any other instance request would not go to this compute | |
| 14:40:49 | sean-k-mooney | if you are using more then is available you can nolonger allocate untill you drop below the new limit | |
| 14:40:55 | sean-k-mooney | but nothing breaks | |
| 14:40:59 | stephenfin | So it'd be the same here, right? | |
| 14:41:01 | bauzas | exactly what I said :) | |
| 14:41:07 | stephenfin | Or am I missing something? | |
| 14:41:07 | sean-k-mooney | it just prevent new instance going to the node | |
| 14:41:09 | bauzas | actually, that's a good point | |
| 14:41:28 | sean-k-mooney | yes it would be the same | |
| 14:41:30 | bauzas | if the host is oversubscribed, that's fine | |
| 14:41:40 | bauzas | it's just the options mean nothing | |
| 14:43:10 | sean-k-mooney | ok i hate myself for saying this but can we seperate hw:cpu_thread_policy into antoher spec for the removal of that option? | |
| 14:44:13 | sean-k-mooney | it can be replaced with a trait for host with SMT enabled | |
| 14:44:34 | sean-k-mooney | if we agree on that then that one less thing we need to figure out in the cpu spec | |
| 14:46:04 | sean-k-mooney | we will be loosing functionality by doing that but if are not ok with removing that option we have a blocker with the larger spec for cpus in placment anyway | |
| 14:46:09 | stephenfin | sean-k-mooney: Not _really_. I mean, 'isolate' results in extra cores being used and those have to be account for somehow | |
| 14:46:33 | sean-k-mooney | stephenfin: they are in the resouce tracker | |
| 14:46:47 | dansmith | sean-k-mooney: you can't really remove that image property | |
| 14:47:00 | dansmith | you can translate it into something more sane, but it's basically API at this point | |
| 14:47:38 | sean-k-mooney | i personally see value in it but its cause huge issues for cpu in plament | |
| 14:47:42 | dansmith | if you just start ignoring that, everyone's tooling is going to start spinning up instances they think are isolated (or whatever) but arent' and they'll find out when it's too late | |
| 14:47:54 | sean-k-mooney | ya i know | |
| 14:48:05 | sean-k-mooney | we can certely translate it | |
| 14:48:21 | stephenfin | dansmith: The migration path we'd suggested was keeping that but limiting it to hosts with "I don't have hyperthreads" trait set | |
| 14:48:23 | stephenfin | I think | |
| 14:48:25 | sean-k-mooney | to forbined:trait=COMPUTE_SMT | |
| 14:48:36 | stephenfin | Wait, yeah, that ^ | |
| 14:49:05 | dansmith | stephenfin: that's cool, it just can't like .. be removed, like sean-k-mooney was saying :) | |
| 14:49:22 | sean-k-mooney | removed was a bad phasing | |
| 14:49:26 | stephenfin | dansmith: True. We can think about deprecating it in the future though | |
| 14:49:28 | sean-k-mooney | its meaning would change | |
| 14:49:34 | dansmith | I don't | |
| 14:49:34 | stephenfin | It's that or we carry the shim forever | |
| 14:49:35 | dansmith | it's API | |
| 14:49:52 | stephenfin | We deprecate/remove other APIs though? | |
| 14:49:54 | dansmith | you can fail the boot if it's specified as anything if you want | |
| 14:50:04 | dansmith | stephenfin: this is unversioned | |
| 14:50:14 | dansmith | but I think we have to check for it basically forever | |
| 14:50:23 | dansmith | it's also API that's unversioned and spread between nova, glance and cinder | |
| 14:50:37 | stephenfin | Hmm, good point | |