Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-27
21:10:52 mriedem mnaser: you're right, we'll just iterate the cells
21:11:48 mnaser i guess in an ideal world you retrieve list of vms from nova_api, and then generate a subsequent list to each cell with a list of instance uuids to request
21:12:08 mnaser which might even eliminate extra calls if a user is located in one cell
21:12:40 melwitt so in the case of a build request with a instance mapping with cell_mapping = None, it will return build_request.instance, which I'm not sure what will happen if you try to delete that
21:12:54 mriedem mnaser: that's what this is for https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/compute/instance_list.py#L101
21:12:56 melwitt presumably it fails
21:12:57 mriedem and that's what cern uses
21:13:39 mnaser wouldn't it be safer to only delete the build request once the cell has been set?
21:13:54 melwitt so that means build_request.instance gets passed to compute API().delete
21:14:54 mriedem melwitt: in that case we should go through here https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/compute/api.py#L1877
21:15:39 mriedem mnaser: the idea is if the user deletes the build request before the instance has been scheduled to a cell, we never create the instance in the cell,
21:15:47 mriedem so there is nothing to do with the instance mapping b/c it's not in a cell
21:16:00 mriedem and shouldn't get listed either b/c it's (1) not a build request and (2) not in a cell
21:16:28 mnaser yeah so maybe the issue here really inside list?
21:16:40 melwitt right, so the delete of the build request would succeed, but then the lookup of the instance will fail because it was just a build_request.instance shell
21:16:41 mriedem which if that is really working, we get here in conductor after the build request was deleted in api https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/conductor/manager.py#L1243
21:17:39 melwitt or well, maybe not. _lookup_instance would return None, None in the cell_mapping = None case
21:17:45 mriedem i wonder why we don't update the instance mapping right after this https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/conductor/manager.py#L1257
21:18:57 mriedem melwitt: right, if _delete_while_booting returns True, we exit https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/compute/api.py#L1877
21:19:10 melwitt hm, so I'm not seeing how delete would fail in that case
21:22:28 melwitt mnaser: is there any chance the service version in one of the records in the 'services' tables is < 15?
21:22:49 mriedem heh, i asked that last week too :)
21:22:50 mnaser melwitt: i checked that with mriedem last time we tried to look into this and no, none
21:22:50 melwitt I guess that wouldn't make sense. all of your instance GET would fail in that case
21:22:56 mriedem btw, i thin kwe should probably remove that service version check now
21:23:12 mriedem commented on the bug https://bugs.launchpad.net/nova/+bug/1784074/comments/1
21:23:12 openstack Launchpad bug 1784074 in OpenStack Compute (nova) "Instances end up with no cell assigned in instance_mappings" [Undecided,New]
21:23:19 mriedem with what *might* be happening
21:23:22 mriedem but you'd have errors in the logs
21:23:25 melwitt this doesn't make any sense how delete returns 404
21:23:50 mriedem melwitt: read ^ that comment in the bug because i think that could explain a window where it could happen
21:24:25 mriedem mnaser: i wonder if these are instances getting created as part of a multi-create request where they all get created in a cell, then when we go to update mappings, something fails and then the rest are left unmapped
21:24:46 mriedem the user attempts to delete the instance, they delete the build request, but then they can still list it,
21:24:55 mriedem but can't delete it b/c the build request is gone and the instance mapping isn't poining at a cell
21:24:58 mriedem hence your fix up script
21:25:03 melwitt ohhh
21:25:34 mriedem this goes back to something we've talked about before where the schedule_and_build_instances method was split into a few phases where it was originally one
21:26:14 mriedem so now we (1) get hosts from scheduler (2) create instances in cells (3) recheck quota (4) do some other stuff including updating instance mappings and casting to compute to build
21:26:27 mriedem if anything fails in the loop in #4 we'd have this situation
21:26:53 mnaser these could be a multi create
21:26:55 mnaser let me double check
21:27:05 mriedem mnaser: you'd have to find the request spec and look that up
21:27:10 melwitt yeah, gosh
21:27:17 mnaser i know of a customer that uses this feature all the time
21:27:23 mnaser so it could just be them
21:27:25 mriedem there should be a num_instances field in the request spec for any of those instances
21:27:40 mnaser nope, at least one i randomly picked out is not a multi create
21:27:51 mriedem ok, well,
21:27:56 mriedem i think the theory still applies
21:28:10 mriedem if we fail *before* setting the instance mapping but after we've created the instance in the cell, we're toast
21:29:47 mriedem did we ever figure out if rabbit being down for notifications could screw us up too? because we send notifications before we update the instance mapping...
21:30:19 melwitt I don't know
21:31:23 mriedem i'll throw something up quick before i have to head out
21:32:00 mnaser so my audit script helped bring them from 20k down to 308 left which have no build_requests, no cell_id in the mapping
21:32:26 mnaser and not existing in any cells
21:32:50 mriedem mnaser: ok those are likely just instance mappings for deleted and purged instances
21:32:56 mriedem do you archive/purge the cell dbs often/
21:32:57 mriedem ?
21:33:25 mriedem b/c it wasn't until i think rocky that we added instance mapping and reqspec hard delete to nova-manage db archive_deleted_rows when instances are archive
21:33:31 mriedem or maybe you run your own archive/purge script?
21:33:31 mnaser select created_at from instances order by id asc limit 1; => 2014-12-14 02:38:53
21:33:33 mnaser ...ha.
21:34:35 mnaser but i think i'm mostly waiting for the rocky archive delete stuff
21:41:10 mriedem do you run your own archive script or nova-manage db archive_deleted_rows?
21:42:33 mnaser mriedem: none of the above, we just have a really really really big database
21:42:53 mnaser mysql indexing seems fast enough that it hasn't really affected us much other than just.. being a big db.
21:42:54 sean-k-mooney mriedem: fyi i left a comment on the review but is the call to self.driver.cleanup in https://review.openstack.org/#/c/586568/1 against the source or dest node?
21:44:20 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Update instance mapping as soon as instance is created in cell https://review.openstack.org/586713
21:44:22 mriedem mnaser: melwitt: throwing things at the wall ^
21:44:41 mriedem sean-k-mooney: source
21:44:49 mriedem _post_live_migration and _rollback_live_migration run on the source host
21:45:43 mriedem sean-k-mooney: replie
21:45:45 mriedem *replied
21:45:52 sean-k-mooney mriedem: oh ok then yes it proably should have the source vif then however i dont think it actully will need them unless we replug the vifs
21:46:02 mriedem that's not what you said last night
21:46:09 mriedem something something ovs hybrid plug cleanup
21:46:19 mriedem but it was 4am and you were maybe loopy
21:46:20 sean-k-mooney mriedem: for the cleanup
21:47:31 sean-k-mooney mriedem: self.driver.post_live_migration_at_source shoudl use the old source vifs so it can unplug correctly
21:48:25 mriedem sean-k-mooney: yes, same thing
21:48:32 sean-k-mooney i dont know what self.driver.cleanup does. if its on the source however it should also proably be using the source vifs
21:48:50 mriedem sean-k-mooney: in the commit message, i pointed out that if post_live_migration_at_source is successful, destroy_vifs=False and the libvirt driver won't try to unplug in cleanup()
21:49:00 mriedem however, not all virt drivers adhere to that destroy_vifs flag
21:49:04 mriedem the hyperv driver doesn't for example
21:49:22 sean-k-mooney ah ok then yes that all looks good then
21:49:30 mriedem it looks...beautiful
21:50:33 sean-k-mooney normally i like shorter fuction names but the at_source and at_destination really helps keep context in this code
21:51:34 mriedem that's why i did https://review.openstack.org/#/c/551371/
21:51:53 mriedem because knowing wtf is going on in the 20 methods involved in live migration is not sometihng you can keep in your head
21:52:12 mriedem also https://docs.openstack.org/nova/latest/reference/live-migration.html
21:52:55 melwitt yes. every time I figure out code like that, a few months later I end up wishing I had added a lot of code comments to it, if nothing else
21:53:30 mriedem yup also https://review.openstack.org/#/c/496861/
21:53:52 melwitt two thumbs up
21:54:03 mriedem thanks ebert
21:54:18 melwitt looking at your change, trying to remember why the instance.create() was split up from the inst mapping update in the first place
21:54:19 mriedem RIP
21:54:26 mriedem melwitt: the quota stuff
21:54:36 mriedem i can find a review comment where we talked about the split
21:54:41 melwitt yeah, trying to re-remember
21:54:43 sean-k-mooney ya i have that bookmarked i just didnt have see we were still in _post_live_migration. that function does a lot

Earlier   Later