Earlier  
Posted Nick Remark
#openstack-nova - 2019-09-10
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
19:36:58 dansmith sean-k-mooney: did you read the comments?
19:36:59 sean-k-mooney not on the resouce class in train
19:37:06 sean-k-mooney dansmith: no not yet
19:37:10 dansmith sean-k-mooney: maybe do that :)
19:37:51 efried good, now everyone can focus on vpmem
19:37:58 sean-k-mooney ok but since we only support vms with all pinned or all shared cpus with this seriese it does not change things as far as i was aware in train
19:38:50 dansmith before this, we'd limit you to at least your vcpu quopta
19:38:53 dansmith after this, no limits AFAICT
19:39:18 dansmith honestly, I'm not sure how you're going to fix it so it works the same with placement quotas and nova quotas
19:39:18 sean-k-mooney we should still be limiting on flavor.vcpu
19:39:29 dansmith not with placement
19:39:32 dansmith did you read the comments?
19:39:47 sean-k-mooney im trying to find which patch you put it on
19:39:57 mriedem (2:36:21 PM) mriedem: this quota issue https://review.opendev.org/#/c/674895/32/nova/virt/libvirt/driver.py@7457
19:40:00 sean-k-mooney oh but i think i know what your saying
19:40:17 sean-k-mooney we have the option to track cpu quota in placmenet already
19:40:33 mriedem by default we don't use placement for quota usage counting
19:40:34 sean-k-mooney we enabled it by default stien right
19:40:40 mriedem f naw
19:40:46 sean-k-mooney oh we dont
19:40:46 mriedem where is melwitt
19:40:53 dansmith we should be in train I think right?
19:40:56 dansmith meaning,
19:41:00 dansmith we should be turning that on but have't
19:41:02 melwitt mriedem: hiding
19:41:21 sean-k-mooney dansmith: so if we turn that on then yes we have a proablem
19:41:26 sean-k-mooney if we dont we should not
19:41:30 dansmith no
19:41:36 dansmith we have a problem because people could have that on
19:41:43 sean-k-mooney ture
19:41:43 dansmith you know config knobs can be set to not the default right? :)
19:41:45 mriedem cern uses it
19:43:24 sean-k-mooney ok i see the problem. i was aware that was a thing but ya i had not consiered the impact to this
19:43:37 mriedem quota is generally the last thing anyone thinks about
19:44:30 dansmith if CPUs weren't the most expensive and constrained resource in the cloud, then maybe less of an issue, but.... :D
19:45:01 mriedem [quota]/injected_file_content_bytes is a major concern of mine
19:45:38 sean-k-mooney im not sure if sarcasim or if i shoudl feel realy bad for you
19:45:46 mriedem heh, sarcasm
19:46:41 sean-k-mooney dansmith: i always ran out of ram first but ya. i think the only thing we coudl do woudl be to could both inventories. at least short term
19:47:03 sean-k-mooney wehere does that code live
19:47:12 sean-k-mooney e.g. where we check the quota
19:47:14 melwitt enforcing quota on other resource classes (DISK_GB and more) is part of the proposal in the unified limits spec, as I think sean-k-mooney mentioned earlier

Earlier   Later