| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-07 | |||
| 14:54:09 | stephenfin | Yeah, my life has been made significantly harder by having to care about 'resources:VCPU' and 'resources:PCPU' | |
| 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 | efried | stephenfin: I would be fine limiting the use of resources:(P|V)CPU as strictly as possible without breaking backward compat. | |
| 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: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. | |
| 15:22:31 | sean-k-mooney | stephenfin: https://github.com/openstack/nova-specs/blob/master/specs/newton/approved/virt-image-props-boot-override.rst | |
| 15:23:34 | efried | gibi: I guess it's not really relevant until we can tie in whatever piece of hardware is associated with the NUMA node. RP for a NIC PF, kind of thing. Cause today you don't request network VF resources or whatever, right? | |