Earlier  
Posted Nick Remark
#openstack-nova - 2019-08-07
14:53:57 mriedem resource: *is* extra specs
14:54:09 stephenfin Yeah, my life has been made significantly harder by having to care about 'resources:VCPU' and 'resources:PCPU'
14:54:09 sean-k-mooney but yes i think you are right the only time you would override them is ironic
14:54:49 stephenfin I'm seriously considering a big "this isn't doing anything unless you're ironic" warning and moving on
14:54:51 stephenfin efried: ^
14:54:54 mriedem https://review.opendev.org/#/c/637217/
14:55:05 stephenfin There's edge case after edge case
14:55:13 sean-k-mooney mriedem: it is but i mean we have hw:cpu_policy to determin if we translate flavor.vcpu to resouce:vcpu or resouces:pcpu
14:55:26 sean-k-mooney stephenfin: the proably is how do you know its an ironic flavor
14:55:38 mriedem sean-k-mooney: if you're talking about things in the plan in the spec, i haven't read it so don't know
14:56:09 efried stephenfin: what specifically? resources:VCPU/PCPU in extra specs?
14:56:17 sean-k-mooney mriedem: we knew trying to supprot resouces: for influncing the xml generation and assignment woudl be a paid soe we are not supporting it
14:56:18 stephenfin mriedem: tl;dr: if you specify 'hw:cpu_policy=dedicated', you now get 'PCPU' inventory, but we need to care about 'resources:PCPU' in addition
14:56:28 stephenfin efried: yup
14:56:29 sean-k-mooney *pain
14:57:14 sean-k-mooney efried: i would expand it to all resocues generated form flavor.* but the problem is we have no way to know if its an ironic flavor or not
14:57:17 mriedem umm, couldn't you just bork if you have hw:cpu_policy=dedicated and resources:VCPU?
14:57:18 sean-k-mooney so we cant
14:57:51 sean-k-mooney we would still request resocue:PCU=flavor.vcpus in addtion to resources:VCPU
14:57:54 mriedem technically any ironic flavor is hw:cpu_policy=dedicated by default yeah?
14:58:13 stephenfin mriedem: Yup, and that's what I'm doing (ditto for explicit PCPU too). The top issue is flavor.vcpus != resources:PCPU
14:58:15 mriedem i mean you do'nt specify that, it's just implied b/c it's baremetal
14:58:37 sean-k-mooney stephenfin: there is one case where its not equal by design
14:58:38 mriedem you know, mogan wouldn't have this problem....
14:58:56 sean-k-mooney which hw:emulator_threads_policy=isolate
14:59:19 stephenfin Also, 'hw:emulator_threads_policy=isolate' means "consume an extra core and put emulator threads there", so there it's (flavor.vcpus + 1) != resources:PCPU
14:59:23 stephenfin sean-k-mooney: jinx
14:59:40 sean-k-mooney mriedem: for ironic all cpus will still be reported using VCPU i think since we dont plan to update the driver
14:59:55 sean-k-mooney at least not unless someone asks
15:00:07 sean-k-mooney stephenfin: yes
15:00:22 mriedem mnaser: dansmith: looking at this trace again https://logs.opendev.org/33/660333/10/check/openstack-ansible-deploy-aio_metal-ubuntu-bionic/97d8bc3/logs/host/nova-compute.service.journal-23-20-40.log.txt.gz#_Aug_06_22_51_25 and where we hit it https://github.com/openstack/nova/blob/master/nova/compute/rpcapi.py#L422 ,
15:00:36 stephenfin So this is all a massive PITA. TBH, I'd rather just say "don't use 'hw:cpu_policy' or any of those extra specs with 'resources:(P|V)CPU' and kick that can down the road
15:00:43 stephenfin efried: Could we do that for now? ^
15:00:44 sean-k-mooney which if we kill that option entrily and just always map the emulator thread to the shared set in the futre will make your life simpeler
15:00:54 mriedem what do you think about adding a try/except for DBNotAllowed to just log something useful and re-raise, like "this service does not allow direct database access, you should run without the [api_database]/connection configured"
15:01:06 stephenfin sean-k-mooney: I'd be happy to do that but I need a cycle to do it
15:01:30 stephenfin Because we break everything otherwise, assuming the operator doesn't have cpu_shared_set configured everywhere
15:01:31 sean-k-mooney ya
15:01:50 mriedem sean-k-mooney: reported....where?
15:02:11 mriedem sean-k-mooney: for ironic the only inventory on the node should be the custom resource class reported by the driver's update_provider_tree method, which gets the resource class off the ironic node resource
15:02:14 sean-k-mooney mriedem: in placement
15:02:20 sean-k-mooney oh right
15:02:22 mriedem and the flavor should only consume, in placement, those custom resource class allocatoins
15:02:24 sean-k-mooney ya i forgot
15:02:31 mriedem that's why the vcpu/ram/disk are all zero'ed out
15:02:44 mriedem the values for those in the flavor are just cosmetic
15:02:57 sean-k-mooney so no change for ironic and hw:cpu_policy will have no effect
15:03:06 sean-k-mooney it should not be used but its meaning less
15:03:21 mriedem mnaser: if you have a bug for that osa issue i could tack on a nova logging patch to maybe make that more clear if someone hits that misconfig
15:04:37 melwitt mriedem: will do
15:05:00 mnaser mriedem: i think noonedeadpunk will likely look into fixing it so might be good if he created a bug to track it ^
15:05:00 efried stephenfin: I would be fine limiting the use of resources:(P|V)CPU as strictly as possible without breaking backward compat.
15:05:18 efried but resources$S:* will be a different story.
15:05:44 stephenfin efried: What would the reason for 'resources$S:PCPU' be?
15:05:57 stephenfin I mean, without NUMA at least
15:06:07 stephenfin *NUMA in placement
15:06:58 dansmith mriedem: sure
15:07:21 efried stephenfin: Exactly NUMA in placement.
15:07:38 efried "will be" future tense intended
15:07:58 sean-k-mooney efried: why not block resources$S:* for pcus and vcpus
15:08:09 efried for now, yes, we certainly can ^
15:08:15 stephenfin Why not forever?
15:08:17 sean-k-mooney we shoudl only support using the hw:numa_cpu.0 values
15:08:48 sean-k-mooney efried: e.g. we already have syntax for specify the cpu to numa mappings
15:08:51 efried stephenfin: pretty sure there will be affinity use cases where we'll need that level of control.
15:08:59 stephenfin Given that we have ways to request all these things with extra specs
15:09:10 efried including things like accelerators?
15:09:15 efried and VGPUs?
15:09:21 efried And bandwidth?
15:09:31 efried esp. that last one
15:09:38 efried because we need to be able to do affinity by subtree matching
15:09:39 sean-k-mooney if we also support mixed cpus in instance we will jsut combien the mask with the hw:numa_cpu.X=Y status
15:09:46 sean-k-mooney *spec
15:09:54 noonedeadpunk mnaser: I guess writing a patch might be quicker that filing in a bug...
15:09:56 efried ...when the requests come from different thingies
15:10:07 huaqiang sean-k-mooney: morning
15:10:47 stephenfin efried: 'hw:numa_affinity:$RESOURCE_CLASS:$POLICY' or something similar?
15:11:26 stephenfin e.g. 'hw:numa_affinity:VCPU:prefer'
15:11:30 stephenfin *VGPU
15:11:52 gibi I don't think we ever want to request bandwidth via flavor exta_spec
15:11:59 efried exactly gibi
15:12:12 efried but we still need to be able to express (NUMA) affinity
15:12:31 stephenfin We're not. We're just saying what the affinity of whatever the bandwidth resource class is
15:12:40 stephenfin Assuming we can know that ahead of time (idk)
15:12:48 efried by tying together the port bw request group suffix and the NUMA request group suffix
15:13:32 efried there's going to be a coupling problem for sure
15:13:38 efried like what we have for group_policy, only much worse.
15:17:24 gibi if nova start using the port uuid as the port suffix for the bandwidth request then a single use flavor can be created where the port group can be referenced for affinity. But this sounds pretty horrific
15:17:37 gibi s/port suffix/group suffix/
15:18:09 efried agree, I don't think we want the suffix to be a port UUID, do we?
15:18:36 gibi efried: currently each port's resource request is understood as a separate group
15:18:52 gibi efried: so a natural suffix for the group could be the port uuid
15:19:12 sean-k-mooney stephenfin: why woudl we need hw:numa_affinity:VCPU:prefer
15:19:26 stephenfin sean-k-mooney: *VGPU
15:19:43 sean-k-mooney oh for graphics ya
15:20:12 stephenfin Just some way to define NUMA affinity for individual resource class types
15:20:24 sean-k-mooney cpus and memory are are affinites by adding hw:numa_node or one of the option that imples it so i was confused
15:20:27 stephenfin It doesn't allow you to define the same for individual instances, but that's slightly insane
15:20:34 stephenfin Yup, my mistake
15:21:19 sean-k-mooney well you would be surprised what we have approved in the past and never implmeneted.

Earlier   Later