| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-10 | |||
| 18:54:14 | donnyd | yea I had them when i was in CoSprings and their business class was pretty good | |
| 18:54:14 | dansmith | but then moved outside their area and they mostly stopped expanding it | |
| 18:55:58 | sean-k-mooney | artom: sorry this took so long but the latest version is updating the numa toplogy blob in the db correctly | |
| 18:56:22 | sean-k-mooney | so puting back in the instance.refresh() or whatever you did fixed that issue | |
| 18:58:35 | sean-k-mooney | im going to try testing a bunch of different cases but are there any in partcalar people want me to test | |
| 19:05:26 | mriedem | artom: so don't respin now since we need to get a result from that ci job, but queue this locally https://review.opendev.org/#/c/634606/83 | |
| 19:06:28 | artom | mriedem, ack | |
| 19:06:32 | artom | sean-k-mooney, cool, thank you :) | |
| 19:07:07 | artom | I managed to hit it func tests, as I said. But... that was by cheating and setting do_cleanup = True in the code itself, to trigger the driver.cleanup() call | |
| 19:08:25 | mriedem | smells like a networking orgy in here | |
| 19:08:33 | artom | So I'm trying to do it correctly by forcing making is_shared_instance_path false | |
| 19:08:42 | artom | Which leads to a whole other rabbit hole... | |
| 19:08:55 | dansmith | mriedem: efried: not sure if you saw this, but alex_xu confirmed that we just stop caring about quotas on pcpu instances (IIUC) per my question about how it works: https://review.opendev.org/#/c/674895/32/nova/virt/libvirt/driver.py | |
| 19:09:27 | mriedem | stephenfin said something about dealing with quota needs to happen yet | |
| 19:09:38 | dansmith | mriedem: efried: I can see the solution being a new quota class (ick) or lumping them together (which may be confusing) but just leaving them ignored doesn't seem like a good plan to me, especially since it differs for the user based on whether or not the deployment is configured for placement | |
| 19:11:10 | sean-k-mooney | dansmith: for what its worth i point out the PCPU quota issue in the unified limts spec | |
| 19:12:03 | sean-k-mooney | if we implement it next cycle we either will have 2 quotas or have to use both vcpu and pcpu count when looking at the cpu quota | |
| 19:13:12 | dansmith | we have to do something other than just pretend they're not there regardless of when we land it | |
| 19:13:41 | sean-k-mooney | well we did not intend to pretend they are not there | |
| 19:13:49 | dansmith | since it's all about consuming whole cpus, I'm pretty sure that operators won't be okay with just allowing anyone to boot enough pcpu guests to exhaust the whole deployment | |
| 19:13:50 | sean-k-mooney | but yes | |
| 19:13:55 | dansmith | I know, but... | |
| 19:14:37 | sean-k-mooney | personally i prefer having two spereate quotas one per RC but i can see why some wont want to distiguiush | |
| 19:15:14 | dansmith | yeah, I mean, | |
| 19:15:22 | dansmith | you'd think that the operators would want to quota those separately | |
| 19:15:36 | sean-k-mooney | so they can bill for the seperatly | |
| 19:15:45 | dansmith | no, | |
| 19:15:46 | sean-k-mooney | because one cost them a lot more | |
| 19:15:57 | dansmith | they will bill separately, they just need to make sure one tenant doesn't eat them all up | |
| 19:16:08 | sean-k-mooney | well ya that too | |
| 19:16:32 | sean-k-mooney | if we treat them sepatly we dont need to special case for them | |
| 19:16:51 | sean-k-mooney | its just 1 limit per resource class | |
| 19:16:59 | sean-k-mooney | so its simpler to reason about | |
| 19:17:30 | sean-k-mooney | the down side is we need to teach people that there would now be two limits on cpus | |
| 19:18:09 | dansmith | well, only if they configure flavors with those things | |
| 19:18:28 | dansmith | I can't imagine anyone that enables this functionality will be fine with treating them as the same.. one costing like 100x the other :) | |
| 19:18:29 | sean-k-mooney | by those things you mean cpu pinning | |
| 19:19:37 | dansmith | no, I mean allowing people to ask for dedicated cpus | |
| 19:19:40 | dansmith | isn't that the point of this? | |
| 19:19:58 | sean-k-mooney | of pcpu in placmenet or unified limits | |
| 19:20:50 | sean-k-mooney | my understanding is we are sitll going to request dedicated cpus the same way we always didn with hw:cpu_policy=dedicated | |
| 19:21:08 | dansmith | I think I've lost grasp of this conversation | |
| 19:21:14 | sean-k-mooney | me too | |
| 19:21:19 | dansmith | heh | |
| 19:21:29 | sean-k-mooney | i was going to say i dont think the way we request things is going to change | |
| 19:21:45 | mriedem | but but but i could override my flavor vcpu with resources:VCPU=0 and define resources:PCPU=1 | |
| 19:22:09 | sean-k-mooney | i am 99% sure stephen add a check in his code to block that | |
| 19:22:18 | sean-k-mooney | we definetly discussed addint one | |
| 19:22:40 | sean-k-mooney | but yes today with out his patches you could | |
| 19:22:42 | mriedem | it would be confusing anyway since flavor.vcpus would be...what? | |
| 19:22:58 | sean-k-mooney | flavor.vcpus would be 1 | |
| 19:23:10 | mriedem | even though you're not getting vcpu, you're getting pcpu | |
| 19:23:15 | sean-k-mooney | yes | |
| 19:23:38 | dansmith | mriedem: right, I'm hoping we go in the direction of a resource override in the flavor and not the hw:$foo stuff | |
| 19:23:43 | mriedem | yeah you can't even create a flavor with vcpu=0 | |
| 19:24:00 | sean-k-mooney | the placement resouce class shouldn nver have been vcpu. flavor.vcpu means virtual cpu count exposed to the guest | |
| 19:24:22 | sean-k-mooney | dansmith: we are currently not going in that direction | |
| 19:24:45 | sean-k-mooney | dansmith: we are currently explcitly planning ot block resouce class overrides | |
| 19:24:50 | dansmith | mriedem: yeah, but the "rewriting" patch will cause you to get an allocation with vcpu=0, which is weirdish | |
| 19:24:55 | dansmith | sean-k-mooney: huh? | |
| 19:25:07 | dansmith | I dunno who the "we" is in that scenario | |
| 19:25:23 | mriedem | we = stephen and the people approving the changes | |
| 19:25:26 | dansmith | do you mean specifically for PCPU things? | |
| 19:25:29 | dansmith | mriedem: heh, yeah | |
| 19:25:37 | sean-k-mooney | well yes to both | |
| 19:25:44 | dansmith | I definitely don't agree with blocking resource based overrides in the future :) | |
| 19:26:01 | sean-k-mooney | erric stephen alex gibi and i were talking about this about a mothn ago | |
| 19:26:23 | sean-k-mooney | dansmith: the reason is if we dont you need to modfiy your flavor if the toplogy of resouce changes in placmenet | |
| 19:26:40 | sean-k-mooney | e.g. you flavor would break if we move cpus under numa nodes or cache nodes | |
| 19:26:42 | mriedem | i get the reason in the short term | |
| 19:27:00 | dansmith | sean-k-mooney: I don't see what that has to do with it at all | |
| 19:27:26 | sean-k-mooney | you woudl have to chagne teh resouce: syntax to the numberd group form | |
| 19:27:34 | efried | I'm not fully swapped into this conversation, but last time I looked you get an error if flavor.vcpu != PCPU | |
| 19:27:49 | efried | (when PCPU is specified) | |
| 19:28:14 | sean-k-mooney | am that is not always correct | |
| 19:28:15 | dansmith | which is also really weird and confusing | |
| 19:28:16 | efried | except for something something hyperthread | |
| 19:28:30 | efried | yes, it's confusing, but it's the compromise that was agreed on in the spec. | |
| 19:28:42 | sean-k-mooney | except if you have hw:emulator_threads_policy=isolate | |
| 19:28:56 | efried | then you add 1 | |
| 19:28:57 | efried | right? | |
| 19:29:00 | sean-k-mooney | in that case we allocate 1 addtional pcpu for the emulater thread | |
| 19:29:02 | sean-k-mooney | yes | |
| 19:29:03 | efried | that was in there too. | |
| 19:29:49 | dansmith | well, I don't see how we can be landing any of this without the quota bit in place at the very least | |
| 19:29:50 | mriedem | which is because of this right? https://github.com/openstack/nova/blob/stable/stein/nova/virt/libvirt/driver.py#L852 | |
| 19:30:16 | mriedem | is hw:emulator_threads_policy and that +1 thing for pcpu restricted to the libvirt driver or does that logic creep into the api? | |
| 19:30:59 | sean-k-mooney | am well that is hard to answer | |
| 19:31:04 | mriedem | GREAT | |
| 19:31:09 | sean-k-mooney | only the libvirt driver supprots pinning | |
| 19:31:14 | sean-k-mooney | and this only works with pinning | |
| 19:31:24 | sean-k-mooney | so this only works with the libvirt dirver | |
| 19:31:32 | dansmith | that is not the right answer | |
| 19:31:50 | dansmith | letting stuff like that creep into the API because only one driver supports it is how we have a ton of xen-specific warts on the api | |
| 19:31:56 | sean-k-mooney | is the right answer we hope to remove that in U | |
| 19:32:23 | sean-k-mooney | because we hope to remvoe it in U | |
| 19:32:36 | mriedem | delicious agent builds | |
| 19:32:39 | sean-k-mooney | now that we have support for share | |
| 19:33:14 | sean-k-mooney | wehich mapps the emulator threads to the same cpu pool as the floating vms we dont think we need isolate anymore | |
| 19:33:33 | mriedem | sean-k-mooney: so by remove "that" you mean hw:emulator_threads_policy ? | |