Earlier  
Posted Nick Remark
#openstack-nova - 2019-09-10
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 ?
19:33:41 sean-k-mooney yes
19:34:44 sean-k-mooney what stephen and i woudl like to propose in U is this. if you have cpu_shared_set defiend the emultor thread run there. if not they run on the same cores as your pinned vm
19:34:59 sean-k-mooney that is what you get if you do hw:emulator_threads_policy=share today
19:35:26 sean-k-mooney and if we only support 1 value for the option we can remove it entirly and just do that by default
19:36:02 mriedem efried: so you might want to not drop the -2 on https://review.opendev.org/#/c/671793/23 until the quota issue is sorted out
19:36:18 mriedem this quota issue https://review.opendev.org/#/c/674895/32/nova/virt/libvirt/driver.py@7457
19:36:30 efried done
19:36:37 sean-k-mooney do we have quota issue wiht this?
19:36:48 sean-k-mooney i though quota was being counted on flavor.vcpu

Earlier   Later