| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-11 | |||
| 21:22:58 | mriedem | but...are we doubling today? | |
| 21:23:07 | mriedem | in the scheduler when we evacuate we must be doubling | |
| 21:23:12 | dansmith | if we are it's not for any useful reason I think | |
| 21:23:25 | mriedem | i think it's just because we do it generically | |
| 21:23:28 | dansmith | I think we delete the allocation for the old node first anyway right? | |
| 21:25:25 | dansmith | allocate _for_evacuate_dest_host | |
| 21:26:27 | mriedem | that's when we're forcing the host during evacuate and bypassing the scheduler | |
| 21:26:33 | dansmith | yeah, we end up calling claim_resources and doubling | |
| 21:26:45 | dansmith | no, that method calls the scheduler | |
| 21:26:57 | mriedem | _allocate_for_evacuate_dest_host? | |
| 21:26:59 | mriedem | it doesn't | |
| 21:27:08 | mriedem | it calls scheduler utils | |
| 21:27:11 | dansmith | https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L782-L782 | |
| 21:27:16 | mriedem | b/c we do the same util thing for live migration with a forced host | |
| 21:27:17 | dansmith | which calls claim_resources | |
| 21:27:29 | dansmith | sorry, maybe it does't call scheduler, but it does the doubling, which is what I meant | |
| 21:27:30 | mriedem | right, that's not select_destinations | |
| 21:27:33 | mriedem | yeah | |
| 21:27:39 | dansmith | sure, I was focused on the allocs | |
| 21:27:44 | mriedem | this is the thing i want to remove with a skip_filters flag to select_destinations | |
| 21:27:56 | mriedem | so getting back to my original question i guess, | |
| 21:27:59 | dansmith | regardless, I don't think this affects the new-world path | |
| 21:28:05 | mriedem | we never remove the allocation from the source node for an evacuation | |
| 21:28:15 | mriedem | except when the source node comes back, if it does | |
| 21:28:18 | openstackgerrit | Hongbin Lu proposed openstack/nova master: placement: add API reference for create inventory https://review.openstack.org/511342 | |
| 21:28:29 | dansmith | you think we never do it now? | |
| 21:28:47 | dansmith | or you think we never will with the new-world way? | |
| 21:28:51 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L680 | |
| 21:28:55 | dansmith | ah, maybe your point is, | |
| 21:29:07 | mriedem | it's going to be a weird side thing | |
| 21:29:08 | dansmith | in the new world way we'll still end up calling the doubler? | |
| 21:29:15 | mriedem | it's a move operation where the source allocation isn't on the migration uuid | |
| 21:29:43 | mriedem | unlike live migrate, cold migrate/resize | |
| 21:29:43 | dansmith | can't we just stop doing the doubling across the board? | |
| 21:29:50 | dansmith | just delete it before we call claim_resources | |
| 21:29:57 | mriedem | for evac? | |
| 21:30:01 | dansmith | that should work for both old and new paths | |
| 21:30:02 | dansmith | yeah | |
| 21:30:08 | mriedem | well, unless evac fails on the dest | |
| 21:30:13 | mriedem | and the instance never moved | |
| 21:30:15 | dansmith | what does it matter? you're not going back | |
| 21:30:34 | dansmith | we've created the migration, the source node is going to nuke it when it wakes up, per the rules | |
| 21:30:42 | mriedem | not necessarily | |
| 21:30:43 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L643 | |
| 21:30:56 | mriedem | the source only cleans up if the move completed, or is in progress | |
| 21:32:03 | dansmith | um | |
| 21:32:24 | dansmith | the whole point of that robustification was to not do that, | |
| 21:32:32 | dansmith | else we'll race with the operation finishing | |
| 21:32:46 | dansmith | I don't see where we're setting =accepted anymore actually | |
| 21:32:53 | mriedem | conductor | |
| 21:32:56 | mriedem | er api | |
| 21:33:00 | dansmith | I don't see it | |
| 21:33:11 | dansmith | ah I see | |
| 21:33:20 | mriedem | yeah so the api creates the migration record in 'accepted' status, | |
| 21:33:22 | dansmith | right, so that happens synchronously with the api call | |
| 21:33:28 | dansmith | right? | |
| 21:33:29 | mriedem | if the evac is successful on the dest, the status goes to 'done' | |
| 21:33:46 | dansmith | so if the evac api call returns, you know it's never starting on the source again right? | |
| 21:34:00 | dansmith | we do that before we create the new allocation, | |
| 21:34:05 | dansmith | so if we have saved that in the db, | |
| 21:34:10 | dansmith | we know we can delete the old allocation without caring | |
| 21:34:19 | dansmith | because at that point, the source host will never take it back | |
| 21:34:27 | mriedem | not necessarily :) | |
| 21:34:56 | mriedem | if conductor/scheduler fails to find a host, the migration status for the evac is marked 'error' | |
| 21:35:02 | mriedem | and the instance is still on the source | |
| 21:35:05 | mriedem | with no allocation on the dest | |
| 21:35:16 | mriedem | https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L871 | |
| 21:35:26 | mriedem | this is all related to that thing gibi and i were talking about before the ptg | |
| 21:35:33 | mriedem | handling allocations for failed evacs | |
| 21:35:41 | dansmith | right and I was trying to say we shouldn't ever let it go back | |
| 21:35:54 | dansmith | so we should include error in the list of states we use in compute right? | |
| 21:36:20 | dansmith | because the recovery path should be re-rebuilding that instance | |
| 21:36:58 | mriedem | rebuilding the instance on the source won't recreate the allocations in placement | |
| 21:37:12 | mriedem | b/c rebuild skips the scheduler, which skips the claim | |
| 21:37:24 | dansmith | okay, I guess, yeah | |
| 21:37:26 | dansmith | regardless, | |
| 21:37:37 | mriedem | so if you didn't move the instance, but you delete the allocations on the source when starting up, rebuild won't help you with the allocations | |
| 21:37:49 | dansmith | I really think we should avoid the source coming back up and re-owning the instance because I think it's going to be a likely source of races | |
| 21:38:31 | dansmith | the people that use this for HA stuff hammer on this pretty hard from scripts and I think the behavior needs to be as predictable and linear as possible | |
| 21:39:51 | mriedem | ok, well, it seems that requires more thought to determine if we change how evac works if the source comes back up and the instance didn't move, or if we should move source node allocations to a migration record during evac, | |
| 21:40:03 | mriedem | all things i can't really sort out in my head right now with maya in my office reading her homework... | |
| 21:40:28 | dansmith | so, we don't clear the instance.host in evacuate for some reason | |
| 21:40:48 | mriedem | we just update it once evac is successful | |
| 21:41:03 | dansmith | if we did, it would make it easier to just let people retry with an evac again if it failed the first time | |
| 21:41:09 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2841 | |
| 21:41:14 | dansmith | yeah | |
| 21:41:34 | mriedem | you can already retry an evac if the first one failed | |
| 21:41:43 | dansmith | not if thecompute host came backup | |
| 21:41:53 | dansmith | because we still have instance.host set and require it be down | |
| 21:42:01 | dansmith | right? | |
| 21:42:12 | dansmith | the source I mean | |
| 21:42:23 | mriedem | well, if we nulled out instance.host, shit gets all sorts of wonky because you then have to deal with local delete in the api bullshit | |
| 21:42:37 | mriedem | wonkified | |
| 21:43:03 | mriedem | i think i've overshot the original problem we were talking about that started this :) | |
| 21:43:21 | mriedem | the assertion that RT doesn't have to care about migration allocations during evac | |
| 21:43:24 | dansmith | so, in your case, | |
| 21:43:39 | dansmith | let's say we evac, fail, leave a doubled allocation today, | |
| 21:43:46 | dansmith | then evac again, what do we do, triple it? | |
| 21:44:49 | mriedem | no, we cleanup the allocations for the failed dest host | |
| 21:44:59 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2831 | |