| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-19 | |||
| 15:41:40 | artom | cfriesen, but it has to remain in the code for older microversions to work | |
| 15:45:47 | johnthetubaguy | mriedem_afk I would have bet on it being after too, I kinda remember alaski attempting to tidy up a bunch of it some time after it landed | |
| 15:47:45 | alaski | tidy up what now? | |
| 15:51:42 | cfriesen | artom: whoops, missed that. | |
| 15:59:21 | dansmith | alaski: boot from volume I think | |
| 16:01:56 | alaski | ahh. Tidy up is being generous. I just made it actually functional for xenserver, and a little more reliable. | |
| 16:02:57 | alaski | you know what would really help, uuids for bdms :) | |
| 16:04:19 | cfriesen | boot from volume definitely came after volume attach/detach | |
| 16:04:53 | cfriesen | (looking at git history) | |
| 16:39:01 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix live migration devstack hook for multicell environment https://review.openstack.org/458537 | |
| 16:39:01 | openstackgerrit | Dan Smith proposed openstack/nova master: De-complicate some of the instance delete path https://review.openstack.org/458636 | |
| 16:39:02 | openstackgerrit | Dan Smith proposed openstack/nova master: Target cell on local delete https://review.openstack.org/464059 | |
| 18:09:49 | cfriesen | dansmith: you around? | |
| 18:13:47 | dansmith | kinda | |
| 18:21:04 | cfriesen | dansmith: if you remember we were talking about the cost of processing network-changed events. it turns out the neutron work to process those events during a migration doubled in Newton when commit 5de902a went in. That was submitted to fix evacuate bugs, but it meant both source and dest compute nodes got the "network-changed" events. | |
| 18:22:36 | cfriesen | I'm writing up a post, but even after mriedem's optimization at https://review.openstack.org/#/c/465787 I'm seeing 4 simultaneous live migrations push neutron up to using 1300% cpu on the controller, with the spike lasting ~45sec | |
| 18:34:38 | dansmith | 128 neutron api calls peg 13 cores for almost a minute? | |
| 18:37:13 | cfriesen | no, more like a few thousand neutron API calls | |
| 18:37:39 | cfriesen | for each port nova queries the fixed IPs, floating IPs, subnets, etc | |
| 18:37:51 | dansmith | oh I see | |
| 18:40:32 | mriedem | cfriesen: so https://review.openstack.org/#/c/465787 does help? | |
| 18:41:41 | cfriesen | mriedem: yes, hugely. for one migration it cut down on the number of neutron API calls by a factor of 6 or so | |
| 18:41:49 | mriedem | wow nice | |
| 18:42:05 | mriedem | ok so i suppose i need to tidy that up | |
| 18:42:15 | mriedem | i can make it smaller by splitting the refactor part into a separate change | |
| 18:42:47 | cfriesen | mriedem: dansmith: I just posted to the openstack-dev list: "massive overhead processing "network-changed" events during live migration". Some hard data in there. | |
| 18:43:08 | cfriesen | mriedem: I also included the change I proposed to query just the one port rather than listing all of them. | |
| 18:47:40 | mriedem | ok i'm post-alient and post-burrito so i need to get my bearings here | |
| 18:50:18 | cfriesen | mridem: (I'm talking about the optimization I proposed in https://review.openstack.org/#/c/465787....I applied it for my testing) | |
| 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 | dansmith | also, when we support cross-cell migrations we'll want network-changed processed on both sides since they're separate databases | |
| 19:24:55 | mriedem | right | |
| 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 | |