| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-22 | |||
| 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 | |
| 14:16:54 | stephenfin | yeah, I've probably gone far enough down this rabbit hole myself | |
| 14:17:02 | mriedem | https://review.opendev.org/#/c/544683/ was the related nova spec | |
| 14:17:17 | mriedem | which was superseded with mel's osc-placement patch | |
| 14:17:38 | mriedem | the aggregates api sugar in nova is that you can set the allocation ratios per aggregate in one call, which you can't do in placement | |
| 14:17:38 | stephenfin | summary: Can't remove ComputeNode.vcpus(_used) or HostState.vcpus(_used) yet because they might be used by external filters... | |
| 14:17:51 | mriedem | and the api right? | |
| 14:17:58 | stephenfin | and the API, correct | |
| 14:18:20 | mriedem | we have talked a few times about the os-hypervisors API, or part of it, just proxying to placement rather than rely on those values set by the RT | |
| 14:18:27 | stephenfin | Might want to put them behind "this field has been deprecated getters/setters, but that's a nice-to-have | |
| 14:19:05 | stephenfin | No one should have (Core|Disk|Ram)Filter enabled so I don't need to worry about those and could probably remove them (it's been long enough) | |
| 14:19:34 | stephenfin | Because of that, I probably don't need to distinguish between vcpus and pcpus in the ComputeNode object and can just lump them in together | |
| 14:20:15 | stephenfin | The os-hypervisors API is already lying about available vcpus (overcommit ratios aren't respected) so what's another lie | |
| 14:20:54 | stephenfin | As you say, could update that to proxy to placement but that's tangential to this and probably shouldn't be lumped into what is already a somewhat large series | |
| 14:21:45 | stephenfin | and I can probably remove the resource claiming stuff for CPU, RAM and disk like dansmith was doing but I need to think of a workaround for handling overhead | |
| 14:22:19 | dansmith | overhead is handled by setting reserved in the compute configs right? | |
| 14:22:37 | stephenfin | Afraid not. XenAPI seems to be adding some amount for each instance booted | |
| 14:23:07 | stephenfin | and that's what we use to account for the extra CPU consumed when you enable emulator thread offloading in Libvirt using the 'isolate' policy | |
| 14:23:12 | dansmith | right, we discussed that, | |
| 14:23:32 | dansmith | and I think we just settled on "meh, set enough reserved to cover enough of your instances" | |
| 14:23:43 | dansmith | in dublin, IIRC | |
| 14:24:06 | stephenfin | I'm happy with that but am I going to break people? | |
| 14:24:52 | stephenfin | i.e. nova will now allow you to boot N+1 instances on a XenAPI host because overcommit is being ignored and the operator forgot to update their reserved | |
| 14:24:58 | mriedem | dansmith: boston was the first time i remember talking about it, but yeah it was awhile ago and the agreed workaround was bumping reserved in config | |
| 14:25:10 | mriedem | jay said he'd write a doc about how to calculate all that but... | |
| 14:27:40 | mriedem | https://bugs.launchpad.net/nova/+bug/1683858 has the details | |
| 14:27:40 | openstack | Launchpad bug 1683858 in OpenStack Compute (nova) "Allocation records do not contain overhead information" [Medium,Won't fix] | |
| 14:28:53 | stephenfin | I'm already changing all the things, so what's another thing to that pile | |
| 14:35:51 | efried | (stephenfin, I'm glad Matt & Dan answered you; I probably wouldn't have been much help there) | |
| 14:37:05 | bauzas | I'm honestly not really paying attention to this channel, but I saw the above discussion | |
| 14:37:14 | bauzas | stephenfin: dansmith: others: any stuff I can help ? | |
| 14:39:40 | stephenfin | bauzas: I think I'm good for now, but keep your ears open :) | |
| 14:42:04 | bauzas | okidoki | |
| 14:43:28 | efried | bauzas: mriedem, or johnthetubaguy: could I please ask you to do the final on https://review.opendev.org/#/c/651681/ ? I was going to proxy sean-k-mooney's +1, but I think that would be three Intels. | |
| 14:43:45 | bauzas | efried: lemme look | |
| 14:43:53 | efried | (auto-converge/post-copy) | |
| 14:45:47 | artom | dansmith, could we get your stamp of (dis)approval on https://review.opendev.org/#/c/671471/ when you get a chance? tia :) | |
| 14:46:37 | bauzas | efried: oh this one ? | |
| 14:46:40 | bauzas | sure, I can +W | |
| 14:46:47 | efried | thanks | |
| 14:46:50 | bauzas | I already reviewed it once | |
| 14:48:23 | openstackgerrit | Bence Romsics proposed openstack/os-vif master: Insert osprofiler trace info as external_ids to the bridge table https://review.opendev.org/665715 | |
| 14:56:47 | mnaser | in nova, has there been some sort of 'standard' on what you do in code that is inherently race-y in the cases that you lose the race to another worker | |
| 14:57:12 | jaypipes | stephenfin: hyperv has a completely different calculation as well for overhead, IIRC. | |
| 14:57:27 | mnaser | in cinder, there's a cast operation that involves a race between workers, and the worker that loses seems to be raising an exception | |
| 14:57:40 | mnaser | which to me seems a little too much, raising an exception for a non-failing scenario | |
| 14:58:57 | efried | mnaser: That sounds really broad to me. I would think one would need to know more about the specific situation. | |
| 14:59:14 | efried | Sometimes you could retry. Sometimes you could log and continue. Sometimes you would have to fail the operation... | |