Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-05-19
18:50:43 mriedem oh i haven't seen comments
18:51:59 openstackgerrit Matt Riedemann proposed openstack/nova master: libvirt: remove glusterfs volume driver https://review.openstack.org/463992
18:52:00 openstackgerrit Matt Riedemann proposed openstack/nova master: libvirt: remove scality volume driver https://review.openstack.org/449364
18:52:10 cfriesen mriedem: my one comment about the number of neutron API calls isn't quite accurate, that's actually the number of lines in the neutron API log...but most API calls resulted in one log line so it should be in the ballpark.
19:00:02 mriedem cfriesen: does the list_ports thing really change much?
19:00:16 mriedem replied to your comments
19:00:21 mriedem either way it's a single API call to neutron
19:00:45 mriedem but if we don't have a nw info cache for the instance, for whatever reason - that shouldn't actually happen, we'd need to go back and pull the entire list of ports to rebuild the entire nw info cache
19:01:20 cfriesen mriedem: that list-ports is getting all the ports in the cloud, right? or is it scoped to the project?
19:01:27 mriedem see the search opts
19:01:35 mriedem it's scoped to the ports owned by that tenant associated with that specific instance
19:01:52 mriedem if we just did show_port w/o those filter parameters, it'd be a possible bug
19:02:04 mriedem because the port might no longer be associated with that instance
19:02:08 mriedem which messes up the logic that comes later
19:02:26 cfriesen mriedem: okay, fair enough. givien that it's associated with the instance it's probably fine. I missed that part.
19:02:31 mriedem ok
19:03:01 mriedem _build_network_info_model is really used for 3 different cases, so it gets really hairy logic-wise
19:03:16 mriedem at some point i think we could split it apart, so we didn't have kwarg soup
19:03:57 mriedem everything in here is pretty fragile though, so it get scared touching most of it, especially since it's related to async event handling
19:04:57 cfriesen the real pain is that we end up getting multiple network-changed events for each port during the live migration. The fact that until Newton we didn't even get the events on the dest compute node makes me think it's not actually needed there.
19:10:51 artom cfriesen, there was https://review.openstack.org/#/c/246910/14 that wanted to send events to both source and dest during live-migration
19:11:05 artom That ended up being re-worked when we merged the other patch
19:11:12 artom But it shows the need was apparently there
19:11:31 artom I suspect a lot of deployments ran with vif_plugging_fatal = false, which masks the whole thing
19:16:57 mriedem replied in the ML
19:17:08 mriedem i'm not sure how sending events to the source compute during an evacuation makes any sense,
19:17:11 mriedem isn't the source compute down?
19:18:44 dansmith it might be, but the point is, any move/migrate should be sending them to both sides
19:19:43 mriedem looking at what artom linked it looked like what was really needed there was the vif plugged/unplugged events, which makes sense
19:20:01 mriedem network-changed is triggered anytime a port is updated - which for us is when it's bound/unbound to a host
19:20:20 mriedem which makes sense, but as noted we shotgun our way to refresh the cache b/c we don't know which specific port was updated in that case
19:20:24 mriedem which is my series of changes
19:20:45 mriedem also,
19:21:04 mriedem i'm loathe to decide in the API based on migration type and event type which we should send and which we shouldn't based on how we expect the computes to behave
19:21:51 artom Also, I thought events the compute didn't specifically set up to listen for got ignored?
19:22:04 mriedem no
19:22:07 artom Is network-changes always processed?
19:22:10 cfriesen sure...but can we in nova selectively ignore the events if we know we don't need to care about them?
19:22:16 mriedem artom: yes
19:22:40 mriedem https://review.openstack.org/#/c/465792/
19:22:43 mriedem oops
19:22:46 mriedem https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6854
19:22:54 mriedem is a flat out, rebuild the entire nw info cache
19:23:20 mriedem https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6862 for network-vif-deleted is targeted to at least just deallocate the port and remove it from the nw info cache w/o a full rebuild
19:23:25 prometheanfire looks like the oslo-cache test may be overly specific http://logs.openstack.org/87/465887/4/check/gate-cross-nova-python27-ubuntu-xenial/19a9161/testr_results.html.gz
19:23:35 mriedem artom: else we fall into https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6800
19:23:47 mriedem _event = self.instance_events.pop_instance_event(instance, event) are the things the specific compute is listening for,
19:23:48 mriedem like vif plugged
19:23:49 artom mriedem, OK, I was thinking of https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L302
19:23:50 dansmith mriedem: yes I think the API making that call is bad news.. it's bad news for even compute manager to do it because the virt driver is running the migration
19:24:21 mriedem ^ triggers the callback into the virt driver to finish spawn()
19:24:33 artom mriedem, oh ok, yeah, your first link shows that network-changed is always processed
19:24:41 mriedem but that's the network-vif-plugged and network-vif-unplugged events
19:24:47 artom Regardless of what the compute prepared for
19:24:55 mriedem right
19:24:55 dansmith also, when we support cross-cell migrations we'll want network-changed processed on both sides since they're separate databases
19:25:22 dansmith if we don't we'll end up with stupid bugs like info cache is as provided by the source compute until the next periodic refresh
19:25:31 mriedem dansmith: during a cross cell migration, wouldn't we get an InstanceNotFound on the source cell when trying to update it's nw info cache?
19:25:32 cfriesen okay, so can we add more info from neutron to reduce the number of queries nova has to do?
19:25:59 dansmith mriedem: no, if we do cross-cell migrations we'll have to create the instance in the target database before any of the migration stuff starts
19:26:11 mriedem ew yeah
19:26:12 artom Also, am I the only one who finds it completely mental that https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6856 is on the *compute*?
19:26:22 artom I'd put that somewhere more central, conductor I guess
19:26:34 dansmith artom: why?
19:26:35 prometheanfire mriedem: need a bug for http://logs.openstack.org/87/465887/4/check/gate-cross-nova-python27-ubuntu-xenial/19a9161/testr_results.html.gz ?
19:26:59 dansmith artom: you have more computes than conductors, and instance events don't come to conductor, they come to the compute(s) that are associated with the instances
19:27:05 cfriesen dansmith: it's a bit silly that both computes are simultaneously querying the same data from neutron
19:27:08 dansmith anything we can obviously do at the edges (computes) is better
19:27:24 mriedem prometheanfire: probably - without me looking into the actual tests
19:27:25 dansmith cfriesen: not in a cross-cell migration
19:27:30 artom dansmith, I guess I was thinking in the context of this overloading Neutron discussion
19:27:42 cfriesen dansmith: wouldn't you have separate conductors there too?
19:27:50 artom dansmith, but yeah, I'll shut up now ;)
19:27:53 prometheanfire mriedem: k, fyi, I think this will be blocking reqs updates on dogpile.cache
19:28:01 dansmith cfriesen: sure? but it's still twice the calls
19:28:17 cfriesen dansmith: for migrations in a single cell you could have the conductor get the data from neutron and then pass just the changed bits to both compute nodes
19:29:06 mriedem network-changed doesn't really have anything to do with the compute
19:29:12 mriedem so that one could be centralized
19:29:22 dansmith cfriesen: yes, you could have the api guess on all that and make those events different
19:29:32 mriedem network-vif-deleted and the vif plug/unplug ones call the driver so that's why they are on the compute
19:30:43 mriedem handling network-changed is an optimization that could be made at the top probably
19:30:48 mriedem but baby steps probably
19:31:38 dansmith mriedem: all events are handled by the computes now, so if you change that, you need to know which conductor (super or cell) to send events to, and then adding new things becomes a little less clear about where each is handled
19:32:10 dansmith making the operations that are happening less insanely heavy will yield much more benefit than cutting a big number in half in some situations
19:32:34 mriedem the former is what i'm trying to start with now yeah
19:32:40 mriedem by targeting the network-changed event to a single port
19:32:45 mriedem so we don't do the full nw info cache rebuild
19:32:48 dansmith yeah
19:33:01 dansmith I thought at one point we had a partial update thing like that
19:33:09 dansmith did we lose that or was it different?
19:33:20 mriedem not sure what you're referring to
19:33:31 cfriesen would it be reasonable for neutron to add more info about what exactly changed? (subnet, fixedip, floatingip, etc.)
19:33:47 mriedem cfriesen: maybe
19:33:54 dansmith we used to rebuild the whole cache every time and arosen did something to try to merge our networkinfo with just one change
19:34:27 dansmith cfriesen: you only get a tag in the event, but you could use that to signal what to refresh, yes, or have a different event like "floatingip-changed"
19:35:03 cfriesen even something like that could cut the number of neutron GETs down dramatically
19:35:03 mriedem https://review.openstack.org/#/q/owner:aaronorosen%2540gmail.com+status:abandoned+project:openstack/nova
19:35:10 melwitt dansmith: I put a lock around the cache access in https://review.openstack.org/465042 to address your comment
19:35:33 dansmith mriedem: no, this thing merged and caused a bunch of bugs, but I thought they all got squashed out
19:35:56 dansmith mriedem: anyway, not worth going to find it, it was 2013ish so just move on optimizing what we have now

Earlier   Later