| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-27 | |||
| 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 | melwitt | I guess that wouldn't make sense. all of your instance GET would fail in that case | |
| 21:22:50 | mnaser | melwitt: i checked that with mriedem last time we tried to look into this and no, none | |
| 21:22:56 | mriedem | btw, i thin kwe should probably remove that service version check now | |
| 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:12 | mriedem | commented on the bug https://bugs.launchpad.net/nova/+bug/1784074/comments/1 | |
| 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 | mnaser | select created_at from instances order by id asc limit 1; => 2014-12-14 02:38:53 | |
| 21:33:31 | mriedem | or maybe you run your own archive/purge script? | |
| 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 | |
| 21:54:57 | mriedem | too much | |
| 21:55:18 | melwitt | I think it was something about, if we failed a quota recheck in the middle of a multi create, and to nix all the instances before creating any mappings | |
| 21:55:25 | melwitt | but we ended up not doing that and putting them in ERROR state | |
| 21:55:45 | sean-k-mooney | part of the issue is ist implementing a state machine and all of that context is mixed in with what its doing | |
| 21:55:58 | melwitt | so that ended up being the wrong thing to do, I think | |
| 21:56:16 | mriedem | melwitt: https://review.openstack.org/#/c/501408/2/nova/conductor/manager.py@1020 | |