| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-22 | |||
| 13:19:14 | sean-k-mooney | yes that is true. | |
| 13:19:18 | alex_xu | efried: yes | |
| 13:19:34 | mriedem | we *could* use migration-based consumers for evac, i just don't think it was really a priority when dansmith implemented that blueprint | |
| 13:20:04 | mriedem | i'm obviously walking into something late here though | |
| 13:20:09 | mriedem | who is out for 2 weeks? | |
| 13:20:13 | efried | gibi_off: | |
| 13:20:31 | efried | mriedem: but that's a separate topic | |
| 13:20:44 | mriedem | ok, which series are you talking about? | |
| 13:21:06 | efried | mriedem: we're talking about alex_xu's work to make hypervisor-specific claiming (atm for VGPUs and VPMEMs) happen in the virt driver. | |
| 13:21:23 | alex_xu | mriedem: https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:claim_for_instance | |
| 13:22:09 | mriedem | ok and this is prep work for vpmem i guess | |
| 13:22:23 | mriedem | claims do happen in the rt today | |
| 13:22:31 | mriedem | the only hypervisor-specific thing though is the overhead stuff | |
| 13:22:57 | alex_xu | mriedem: vgpu allocation has race problem also, in libvirt, we allocate vgpu out of rt, that means no lock | |
| 13:23:45 | efried | also, it just makes sense for hypervisor-specific claiming to happen in the virt driver. | |
| 13:24:31 | mriedem | alex_xu: it happens during driver.spawn correct? | |
| 13:24:41 | efried | So tldr we're adding a hook from the various *_claimZ to call into a new ComputeDriver interface which interprets allocations etc to earmark the real devices corresponding to the allocations | |
| 13:24:42 | alex_xu | mriedem: yes, https://bugs.launchpad.net/nova/+bug/1836204 | |
| 13:24:42 | openstack | Launchpad bug 1836204 in OpenStack Compute (nova) "The allocation of VGPU has race problem" [High,Triaged] - Assigned to Alex Xu (xuhj) | |
| 13:25:43 | mriedem | i guess we're not modeling each specific device in placement right? just the total number of available in inventory, and then it's up to the virt driver to pick one, which could conflict. | |
| 13:26:04 | efried | exactly | |
| 13:26:21 | mriedem | weee, so much for ever getting rid of the rt claims code... | |
| 13:26:59 | efried | The RT claims *framework* will need to stay, but IMO eventually "all" of the guts should wind up moving into this ComputeDriver.claim_for_instance | |
| 13:27:11 | efried | especially for things like PCI devices | |
| 13:27:18 | efried | and NUMA | |
| 13:27:44 | efried | the line where it makes sense to cut things over is when we move to tracking a resource in placement. | |
| 13:28:02 | efried | the messy ones obviously being VCPU, MEMORY_MB, DISK_GB | |
| 13:28:53 | sean-k-mooney | efried: well placement just keeps a tally count. if we need to do device assignment we need to continue to do tracking the the RT | |
| 13:29:03 | efried | yup | |
| 13:29:25 | efried | where "device" will eventually encompass NUMA-split proc/mem | |
| 13:29:50 | sean-k-mooney | ram and cpus are just devices in disgiuse | |
| 13:30:32 | alex_xu | I have too much fun with same host resize today | |
| 13:30:58 | efried | bbiab | |
| 13:36:47 | stephenfin | mriedem, efried: I'm hitting the boundaries of my resource tracking know how and could do with some help | |
| 13:37:03 | stephenfin | I've got this patch to start tracking pcpus as a ComputeNode/HostState field https://review.opendev.org/#/c/671794/ | |
| 13:37:47 | stephenfin | It's not complete, and it's also breaking some functional/tempest tests because one of those objects is being spat out for the os-hypervisors API | |
| 13:38:42 | stephenfin | However, does any of the resource tracking claims stuff matter for vcpus, ram and disk anymore, given we're actually requesting things from placement? | |
| 13:39:25 | openstackgerrit | sahid proposed openstack/nova master: cellv2: make update_cell to support cell0 https://review.opendev.org/672045 | |
| 13:39:28 | stephenfin | Referring specifically to this https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L97-L109 | |
| 13:40:01 | mriedem | they used to matter for scheduler drivers that didn't use placement, like the caching scheduler, but that's gone now, | |
| 13:40:09 | mriedem | the overhead stuff comes from the driver and that's still part of the claim | |
| 13:40:17 | mriedem | i know the libvirt driver claims overhead in certain case | |
| 13:40:18 | mriedem | *cases | |
| 13:40:46 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L884 | |
| 13:40:47 | stephenfin | Yup, for emulator threads, though I do have a chance to stop doing that now with the PCPU stuff | |
| 13:41:08 | mriedem | there are other drivers that implement that method as well | |
| 13:41:10 | mriedem | hyperv i think is one | |
| 13:41:46 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/hyperv/vmops.py#L119 | |
| 13:42:43 | mriedem | besides the overhead stuff i'm not sure anything cares about vcpus/ram/disk claims in the rt anymore since that should be handled by the scheduler + placement now | |
| 13:43:00 | mriedem | dansmith at one time had a patch to rip that out | |
| 13:43:05 | stephenfin | Hmm, so it sounds like that alone prevents me from simply removing this stuff https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L160-L162 | |
| 13:43:28 | mriedem | https://review.opendev.org/#/c/551026/ | |
| 13:43:29 | dansmith | https://review.opendev.org/#/c/551026/ | |
| 13:43:32 | dansmith | heh | |
| 13:44:04 | stephenfin | Okay, I might incorporate that into my series, if that makes sense? | |
| 13:44:33 | dansmith | we never removed cachingscheduler did we? but surely we can now yeah? | |
| 13:44:36 | mriedem | we did | |
| 13:44:41 | openstackgerrit | sahid proposed openstack/nova master: cellv2: make update_cell to support cell0 https://review.opendev.org/672045 | |
| 13:44:41 | mriedem | i just left a comment on dan's patch | |
| 13:44:43 | dansmith | oh sweet | |
| 13:45:28 | mriedem | in boston we talked about the overhead-from-driver problem for claims / placement and i think had mostly just shrugged it off saying as a workaround operators could bump up the reserved amount of inventory on hosts that would use vms that need the overhead space | |
| 13:46:46 | mriedem | caching scheduler was removed in stein fwiw | |
| 13:47:57 | stephenfin | Hmm, okay, I clearly need to think about this | |
| 13:48:28 | stephenfin | Starting with figuring out if anything else is using/cares about the ComputeNode.vcpus/vcpus_used field | |
| 13:48:39 | stephenfin | (and therefore whether I need a pcpu equivalent) | |
| 13:55:00 | stephenfin | mriedem: Can/should 'Aggregate(Ram|Disk|Core)Filter' be deprecated? | |
| 13:55:34 | dansmith | those are widely used I believe | |
| 13:55:46 | dansmith | and don't have placement-based alternatives because they work on aggregates | |
| 13:56:04 | dansmith | we broke the allocation ratio one and people revolted | |
| 13:56:41 | stephenfin | I'm misreading this release note so, I guess :( https://github.com/openstack/nova/blob/master/releasenotes/notes/agg-resource-filters-6e24c92a69afa85f.yaml | |
| 13:56:56 | stephenfin | That suggests to me that aggregate-based overcommit ratios aren't a thing anymore | |
| 13:57:13 | stephenfin | and the filters can be removed because they're useless | |
| 13:57:40 | dansmith | there are more functions in the aggregate filters than just allocation ratios right? | |
| 13:57:49 | dansmith | however, that breakage was what led to the revolt I think | |
| 13:57:55 | dansmith | i.e. that commit | |
| 13:58:51 | stephenfin | Oh, quite possibly :) Looking at the AggregateDiskFilter, I don't see much more happening https://github.com/openstack/nova/blob/master/nova/scheduler/filters/disk_filter.py | |
| 13:59:10 | mriedem | that release note was from ocata i thought, and yeah part of the misunderstanding | |
| 13:59:35 | stephenfin | (Though the __init__.py for same should probably be overridden to not log the warning) | |
| 13:59:41 | mriedem | a more detailed description is in the docs now https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html#allocation-ratios | |
| 13:59:53 | stephenfin | wait, nvm. I see DEPRECATED now | |
| 14:00:27 | dansmith | stephenfin: yeah maybe I'm confused,but I thought there were a couple other aggregate-based operations other than just the ratio for those, but maybe I'm wrong | |
| 14:00:35 | dansmith | or maybe the filters I'm thinking of aren't tied specifically to those three classes | |
| 14:00:41 | mriedem | stephenfin: any out of tree filters/weighers could be using ComputeNode.vcpus/vcpus_used | |
| 14:00:50 | mriedem | plus the os-hypervisors api is using them | |
| 14:01:25 | mriedem | "clearly need to think about this" is probably the understatement of the year for that pcpu overhaul blueprint | |
| 14:01:59 | stephenfin | Fair point. So I probably can't outright remove them, but I'm thinking/hoping I don't need to distinguish between pcpus/vcpus at that particular level | |
| 14:02:01 | stephenfin | Amen. | |
| 14:03:44 | stephenfin | mriedem: fwiw, the doc linked says the exact same thing as the release note and gives me the same impression. If I'm reading it correctly, I assume that "Note" needs to be removed? https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html#scheduling-considerations | |
| 14:06:34 | mriedem | no, | |
| 14:06:49 | mriedem | it's saying the allocation ratio set via the aggregate metadata is broken since ocata, | |
| 14:07:05 | mriedem | and you *have* to set the allocation ratio per the computes / resource providers instead | |
| 14:07:18 | mriedem | which leads into the usage scenarios section | |
| 14:09:03 | mriedem | https://review.opendev.org/#/c/640898/ is also related | |
| 14:09:10 | mriedem | and was something we talked about at the ptg in dublin | |
| 14:10:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional test for resize crash compute restart revert https://review.opendev.org/670393 | |
| 14:10:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass migration to finish_revert_migration() https://review.opendev.org/668631 | |
| 14:10:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: [DNM] testing bug/1813789 revert resize events https://review.opendev.org/664442 | |
| 14:11:56 | stephenfin | I guess (3) is the important piece from that? If you want to use aggregate-based allocation ratios, you *must* set e.g. '[DEFAULT] cpu_allocation_ratio' to 'None' | |
| 14:15:32 | mriedem | those scenarios (1 and 3 specifically) are more about specific use cases when discussing this problem with operators, i.e. cern does everytihng with config mgmt and cares about scenario 1. iweb people exposed the host aggregate metadata stuff to users to control their own allocation ratios so they cared about the api part, which is scenario 3 | |
| 14:16:15 | mriedem | it's been quite awhile since i've had to think about this much, and at this point i don't really know why someone would even enable those filters now | |