Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-31
15:43:38 mriedem jroll: the patch seemed to have some controversy, but it was merged so might as well be backported
15:43:57 mriedem stephenfin: i would need to double check, but i think the only thing we do claims on in the RT w/o the limits dict is the pci requests
15:44:05 mriedem because those were done differently, for whatever reason
15:44:12 mriedem and the inconsistency is hella confusing
15:44:30 stephenfin mriedem: Quick debugging showed the limits dict was always empty
15:44:36 stephenfin Using filter_scheduler, anyway
15:44:42 mriedem stephenfin: for vcpu/ram/disk yes
15:44:50 mriedem the numa filter was, i thought, the only one that put stuff into it
15:44:58 mriedem if using filter scheduler
15:45:12 mriedem but you made some comment in vancouver about how that might not be true, which dansmith ack'ed and i didn't get
15:45:38 mriedem pci_requests are the snowflake right now https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L211
15:46:14 mriedem if we passed through network_requests, then we'd have (1) limits (2) pci_requests and (3) network_requests, all for doing similar claims type stuff
15:46:22 mriedem which makes me want to do bad things, physically
15:46:31 mriedem maybe i'm the only one that feels this way though
15:46:35 stephenfin mriedem: You're referring to https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L102 ?
15:46:42 mriedem stephenfin: yes
15:47:41 stephenfin That does clear things up slightly. I was looking at the limits arg to 'numa_fit_instance_to_host' which is an object, not a dict. Hella confusing
15:48:27 mriedem i think https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L354 is where we take the limits from the HostState after the filters run and shove them into the Selection object which gets passed back to conducotr
15:48:29 mriedem *conductor
15:48:45 mriedem yup https://github.com/openstack/nova/blob/master/nova/objects/selection.py#L53
15:49:36 mriedem and then those are passed to compute here https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L1265
15:49:58 mriedem so unless dansmith feels otherwise, i'd prefer to just continue using the limits dict for now for limits specific things
15:50:06 mriedem rather than add another wrinkle to where we get limits in the RT
15:50:15 stephenfin mriedem: So you're thinking I shouldn't be doing this https://review.openstack.org/#/c/564448/3/nova/virt/hardware.py@1609 right?
15:50:24 stephenfin i.e. no extra parameter
15:50:51 stephenfin instead, do limits['network_requests'] or the likes inside that function
15:51:45 mriedem i can't hardly read that code so i'm not sure what it's doing
15:51:50 dansmith mriedem: I'm on a call right now, but I asserted in our talk that limits is really just a temporary mechanism for communicating decisions from the scheduler filters to the lower layers,
15:51:56 dansmith which this is I think, so I think it fits
15:52:04 dansmith are you agreeing with that or asserting something else?
15:52:14 mriedem dansmith: you and i are in agreement,
15:52:36 mriedem stephenfin found that we pass requested_networks down to compute also, which will have the same type of informatoin in them so we could use that, and i'm asserting we shouldn't b/c it's confusing
15:52:48 dansmith agree
15:53:06 mriedem stephenfin: numa_fit_instance_to_host doesn't modify the limits dict today does it?
15:54:32 stephenfin mriedem: Narp. _numa_fit_instance_cell consumes any NUMATopologyLimits objects contained therein but doesn't modify it
15:54:38 mriedem stephenfin: i don't think that numa_fit_instance_to_host should modify the limits dict, the scheduler filter should do that
15:54:58 mriedem i would basically like to keep all of this as decoupled as reasonably possible
15:55:04 mriedem because right now it's a tightly coupled mess
15:55:33 mriedem and it's like the last part of nova i'm not really familiar with, and don't really ever want to be if i can help it :)
15:56:07 stephenfin By scheduler filter you mean the NUMATopologyFilter?
15:56:10 mriedem yes
15:56:17 mriedem the thing that already puts the numa limits in the limits dict today
15:56:26 mriedem the scheduler filters are the only things that modify the limits dict
15:56:32 mriedem we should maintain that pattern
15:56:56 openstackgerrit Merged openstack/nova stable/ocata: Add ssbd and virt-ssbd flags to cpu_model_extra_flags whitelist https://review.openstack.org/570514
15:57:02 stephenfin We're still going to need some way to get that info into the filter though
15:57:17 stephenfin So we'd still be modifying RequestSpec
15:57:20 mriedem the request spec will have the requested_networks right?
15:57:26 mriedem yes i know and agreed with that already
15:58:22 mriedem api -> modify reqspec.requested_networks like pci requests -> scheduler -> numa filter + modify limits -> conductor -> compute -> RT (claim from limits) -> allocate_for_instance -> update instance info cache with network physnet and tunneled info -> driver.spawn
15:58:24 mriedem done!
15:59:07 stephenfin Oh, so I still have the RequestSpec changes which means for the scheduler I'll just be undoing this change https://review.openstack.org/#/c/564448/3/nova/virt/hardware.py@1609
15:59:13 stephenfin (where I add 'network_requests'
15:59:28 stephenfin ...and instead consuming 'network_requests' from limits
15:59:53 belmoreira mriedem: I don't have the logs for https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L531
16:00:01 stephenfin Then I can use that for the call to 'instance_claim'
16:00:02 stephenfin Gotcha
16:00:06 mriedem stephenfin: i don't really know what that code is doing, so hard for me to say
16:00:17 stephenfin mriedem: I'm just referring to the function signature
16:00:32 mriedem stephenfin: i'm missing the context at which numa_fit_instance_to_host gets called
16:00:34 mriedem i know the numa filter calls it,
16:00:38 mriedem but the RT does also correct?
16:01:01 mriedem well, _test_numa_topology in the claims code, called by the RT
16:01:04 stephenfin mriedem: Yeah, that's here https://review.openstack.org/#/c/564449/3/nova/compute/claims.py
16:01:26 stephenfin I could undo most of those changes
16:01:30 mriedem right so i think you don't need to pass requested_networks in there, just get it from the limits
16:01:36 stephenfin Exactly
16:01:44 stephenfin OK, that clears things up massively. Cheers :)
16:01:51 mriedem do you need to pass anything into numa_fit_instance_to_host from the numa filter?
16:02:06 mriedem stephenfin: heh, yeah, you were over-complicating this it sounds like
16:02:26 stephenfin I'll need to set the 'network_requests' field in limits but that's it
16:02:36 mriedem remember, at any given point in any given location in nova, we have at least 3 random dicts of stuff you can leverage for master hackery
16:02:55 stephenfin mriedem: Yeah, PCI requests probably wasn't the best pattern to copy
16:03:06 mriedem definitely not
16:03:12 mriedem i cringe whenver i see that
16:08:45 openstackgerrit Merged openstack/nova master: Update overriden to overridden https://review.openstack.org/571418
16:10:05 stephenfin jaypipes: What's your thoughts on all the above?
16:11:02 mriedem tssurya: i've added some detailed comments to https://bugs.launchpad.net/nova/+bug/1771806/comments/3
16:11:03 openstack Launchpad bug 1771806 in OpenStack Compute (nova) "Ironic nova-compute failover creates new resource provider removing the resource_provider_aggregates link " [Medium,Confirmed] - Assigned to Surya Seetharaman (tssurya)
16:11:03 stephenfin jaypipes: tl;dr: We're going to stuff the NetworkRequestList object into the limits dictionary like we do for NUMATopologyLimits https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L102
16:11:27 mriedem stephenfin: i didn't think it would be that entire NetworkRequestList object
16:11:42 mriedem i thought it would be actual specific limits things
16:12:04 stephenfin mriedem: A new object so?
16:12:19 mriedem maybe, i'm not sure what your data structure needs to look like for the limits stuff
16:12:37 mriedem but i'm pretty sure it could certainly be stripped down from the full NetworkRequestList right?
16:12:50 stephenfin It can, yeah. I could reuse NUMANetworkInfo https://review.openstack.org/#/c/564439/3/nova/objects/numa.py
16:13:58 mriedem maybe, whatever it is will go into the SchedulerLimits object https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L774
16:14:20 tssurya mriedem: looking
16:14:35 mriedem tssurya: just the same stuff i said in irc, but with detailed links
16:15:47 oomichi alex_xu: are you still online now? happy if you take a look at https://review.openstack.org/#/c/571335
16:19:19 melwitt mriedem: I was thinking queens release will be a minor version bump because of this, do you agree? https://git.openstack.org/cgit/openstack/nova/commit/?h=stable/queens&id=45331d3ea185d08687e9b24e716f3d5411210874
16:21:07 mriedem melwitt: umm,
16:21:29 mriedem i personally don't really see the need for a minor version bump, i don't know exactly what that will signal since we already have a release note
16:21:37 melwitt api change? I wasn't sure
16:21:51 mriedem about the only time i've seen stable branch minor version bumps is when we have to adjust dependencies,
16:21:52 melwitt okay
16:21:56 mriedem or maybe with a new db migration
16:22:04 mriedem the api change is to fix broken behavior
16:22:13 mriedem so 409 rather than screw up your data
16:23:00 melwitt okay, I've got the bugfix version bump proposed at https://review.openstack.org/571494

Earlier   Later