Earlier  
Posted Nick Remark
#openstack-nova - 2018-04-16
16:10:32 mriedem cfriesen: i need more of the stacktrace
16:10:37 mriedem not the middle of it
16:11:10 stephenfin bauzas: Want to weigh in on this comment? https://review.openstack.org/#/c/379525/9/nova/scheduler/weights/cpu.py
16:11:17 cfriesen mriedem: will try to get it.
16:11:24 stephenfin bauzas: We don't do this for RAM or disk. Should we?
16:12:36 mriedem stephenfin: the core filter does
16:12:37 mriedem vcpus_total = host_state.vcpus_total * cpu_allocation_ratio
16:13:56 mriedem stephenfin: also, it looks like the HostState object takes into account reserved_host_cpus?
16:14:21 cfriesen mriedem: I think the core/ram/disk filters account for allocation ratio (though disk is wonky), but maybe the weighers don't?
16:15:08 stephenfin mriedem: *don't take into account?
16:15:42 mriedem yeah doens't take into account
16:15:52 mriedem the inventory on the resource provider for the compute node in placement accounts for reserved values
16:16:03 mriedem but i don't see that on the HostState object or it's wrapped compute node
16:16:18 stephenfin mriedem: Line 1136 of nova/compute/resource_tracker.py ?
16:16:24 cfriesen looks like for ram and disk they just weigh them based on the "free" amount
16:16:26 stephenfin (_update_usage_from_instances function)
16:16:50 stephenfin So I'm guessing it gets it from the ComputeNode object
16:17:52 mriedem stephenfin: hmm, yeah, but that's weird.
16:17:56 mriedem cn.vcpus_used = CONF.reserved_host_cpus
16:18:04 mriedem that's not accounting for the number of instances running on the ohst
16:18:05 mriedem *host
16:18:59 stephenfin mriedem: We do that at the end of the function with the call to '_update_usage_from_instance', I suspect
16:19:12 stephenfin Which presumably appends rather than overwrites
16:19:27 stephenfin cfriesen: Hmm, so as we don't have a "free"-style attribute for CPUs I guess they're correct and we've to figure that out ourselves
16:19:34 stephenfin Or add such an attribute
16:20:51 mriedem stephenfin: yeah looks like _update_usage()
16:21:11 mriedem vcpus_usage += overhead.get('vcpus', 0)
16:21:11 mriedem which also takes into account overhead from the virt driver
16:23:22 mriedem ok left some comments in your change for reference
16:23:32 stephenfin mriedem: Excellent. Ta
16:23:35 mriedem i won't profess to know what the right answer is
16:23:46 mriedem it does match this though https://github.com/openstack/nova/blob/fb0b785169e5e422b06e82f2eb58e68f6d2008b3/nova/compute/resource_tracker.py#L786
16:24:46 mriedem this is the claim code
16:24:47 mriedem https://github.com/openstack/nova/blob/fb0b785169e5e422b06e82f2eb58e68f6d2008b3/nova/compute/claims.py#L185
16:24:53 mriedem which is what ultimately matters
16:26:09 mriedem the fun part is,
16:26:27 mriedem if you're using placement, you shouldn't enable the CoreFilter, which means the vcpus limit isn't passed from scheduler to compute, and no vcpus claim is actually done in the resource tracker
16:26:44 cfriesen mriedem: here's the full trace: http://paste.openstack.org/show/719309/
16:26:48 mriedem the only actual RT claim we should ever have, if you're using placement, right now is for numa
16:26:53 mriedem and maybe pci?
16:27:29 mriedem cfriesen: that's the exact same thing that melwitt fixed
16:27:54 cfriesen mriedem: no, melwitt's change was a reader->writer context thing
16:29:24 mriedem hmm, so more similar to Ic2f239f634f917a5771b0401a5073546c710c036
16:29:30 mriedem except in that case the instance is deleted but the service isn't
16:30:36 mriedem dansmith: didn't you recently have a read_deleted thing in the Instance object for evacuate?
16:30:55 dansmith mriedem: idk
16:31:30 mriedem https://github.com/openstack/nova/commit/619754f5c836ed1b58c807138836e6cf5a4e6904#diff-1b01e8e37be9d889295e5333b7857139
16:31:52 mriedem cfriesen: https://review.openstack.org/#/q/Ide6cc5bb1fce2c9aea9fa3efdf940e8308cd9ed0
16:32:28 mriedem maybe you have that already...
16:34:06 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION for "Rocky" https://review.openstack.org/558783
16:34:07 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Drop MIN_{LIBVIRT_KVM,QEMU}_S390_VERSION https://review.openstack.org/560363
16:34:08 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Drop MIN_LIBVIRT_KVM_PPC64_VERSION https://review.openstack.org/560364
16:34:09 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Drop MIN_LIBVIRT_VIRTUOZZO_VERSION https://review.openstack.org/560365
16:34:25 cfriesen mriedem: nope, don't have it. will try it out. thanks.
16:34:39 kashyap mdbooth: ^ If you want to fetch now.
16:39:01 stephenfin mriedem: If I request 30 CPUs and I only have 22 available, I get a negative number for https://github.com/openstack/nova/blob/fb0b785169e5e422b06e82f2eb58e68f6d2008b3/nova/compute/resource_tracker.py#L786
16:39:11 stephenfin mriedem: Sounds like a bug to me at least
16:40:23 cfriesen stephenfin: I think all of those should factor in allocation ratio somehow
16:40:23 mriedem stephenfin: if you request 30 CPUs and the compute node is reporting it only has 22, then the scheduler should filter out that host and you never hit that compute, right?
16:40:36 cfriesen mriedem: not if cpu_allocation_ratio is 2
16:40:45 stephenfin mriedem: I've 22 * 16
16:41:02 stephenfin At least, assuming I'm understanding overcommit correctly
16:41:22 stephenfin The only thing it should fail on is a single instance having > 22 vCPUs
16:41:29 stephenfin (No self-overcommit)
16:42:01 mriedem sure, misunderstanding. then i guess that's why the CoreFilter cares about allocation ratio yeah? https://github.com/openstack/nova/blob/fb0b785169e5e422b06e82f2eb58e68f6d2008b3/nova/scheduler/filters/core_filter.py#L49
16:42:05 cfriesen when we moved away from per-host-aggregate allocation ratios, it might have made sense to remove all knowledge of allocation ratios from anywhere but the compute node itself and just have it lie about how many it has. :)
16:42:08 mriedem and placement for that matter
16:42:37 stephenfin mriedem: I guess so, yeah. Fair play to Rong Han for spotting it
16:42:54 mriedem maybe he already has that weighter out of tree and is just comparing :)
16:43:08 stephenfin cfriesen: Agreed. Do wonder how that would affect CPU pinning though. Don't we use that?
16:43:23 cfriesen how do we support disk allocation ratios with network disks, when different compute nodes could have different allocation ratios configured?
16:43:31 cfriesen stephenfin: pinning ignores the ratios, always 1:1
16:43:36 stephenfin mriedem: I'd put money on it :)
16:44:03 stephenfin cfriesen: Not in the ComputeNode was reporting N * ratio free CPUs though, right?
16:44:07 mriedem (again)
16:44:22 stephenfin Although that's neither here nor there. I'll fix this up here and then fix up the log
16:44:23 cfriesen stephenfin: pinning doesn't use that count
16:44:27 stephenfin which seems broken
16:44:36 stephenfin cfriesen: Ah, potential clean up so
16:44:51 cfriesen stephenfin: at least, I'm about 90% sure it doesn't. :)
16:48:56 cfriesen jaypipes: do you know how we handle disk_allocation_ratio as a per-compute-node config option when we're using shared disk resource providers?
17:22:41 openstackgerrit Eric Fried proposed openstack/nova master: WIP: placement: Granular GET /allocation_candidates https://review.openstack.org/517757
17:28:00 openstackgerrit Chris Dent proposed openstack/nova master: WIP: Parse forbidden in extra_specs https://review.openstack.org/561677
17:37:53 jaypipes cfriesen: nobody is yet using shared disk resource providers, so I don't know :)
18:04:03 cfriesen jaypipes: seems like we'd need to have the allocation ratio associated with the resource provider itself rather than the compute node
18:07:26 cdent cfriesen: yes, that would be the idea. efried has done some preliminary libvirt work, lemme find the link
18:07:31 cdent also you aware of #openstack-placement
18:07:51 cdent cfriesen: https://review.openstack.org/#/c/560459/
18:08:26 efried cdent, cfriesen: In case you didn't catch the last nova meeting, bhagyashris agreed to take these patches over and fix up the tests.
18:09:51 openstackgerrit Jay Pipes proposed openstack/nova master: tests for alloc candidates with nested and traits https://review.openstack.org/531899
18:09:52 openstackgerrit Jay Pipes proposed openstack/nova master: placement: resource requests for nested providers https://review.openstack.org/554529
18:10:43 jaypipes cfriesen: see https://review.openstack.org/#/c/544683/
18:13:38 openstackgerrit Matt Riedemann proposed openstack/nova master: Document how to disable notifications https://review.openstack.org/561684
18:16:43 dansmith sean-k-mooney: around?
18:21:19 mriedem cdent: thanks for https://pypi.org/project/wsgi_intercept/ - i'll see about getting g-r updated
18:23:09 mriedem although i likely can't use that in this change until later since we might backport this patch
18:23:32 cdent yeah, was worth finding anyway
18:29:24 openstackgerrit Merged openstack/nova master: Don't log a warning for InstanceNotFound with deleted VIFs https://review.openstack.org/554591
18:33:28 sean-k-mooney dansmith: i was on a call but yes
18:33:53 dansmith sean-k-mooney: hey, so we had a question on friday and I think you're the right person to ask

Earlier   Later