| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-11 | |||
| 19:57:25 | melwitt | which makes me think it's a relatively new issue | |
| 20:34:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update "SHUTOFF" description in API guide https://review.openstack.org/510697 | |
| 20:34:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: fix server status values in GET /servers docs https://review.openstack.org/510696 | |
| 20:43:08 | mriedem | uh, filter(~models.Migration.status.in_ means NOT IN right? | |
| 20:43:11 | mriedem | the ~ | |
| 20:47:20 | efried | sdague https://review.openstack.org/#/c/511006/ basically passed - the one failure seems unrelated and I'd rather not choke the already-seemingly-choking gate further. | |
| 20:54:57 | mriedem | dansmith: want to double check my thoughts about migration.status = 'done' in here? https://review.openstack.org/#/c/506419/ otherwise i think it's ok | |
| 21:01:23 | mriedem | this also seems like it could bite us https://github.com/openstack/nova/blob/64635ad4a5f60a79e1ec2d5369a8f84bfeccb7e4/nova/db/sqlalchemy/api.py#L4859-L4862 | |
| 21:01:47 | mriedem | when the RT pulls migration records, it's for all migrations where either the source or dest is our local node | |
| 21:08:13 | dansmith | mriedem: yep will in a sec | |
| 21:20:25 | dansmith | mriedem: replied | |
| 21:22:22 | mriedem | ok didn't think about not having migration allocations for evacs | |
| 21:22:41 | mriedem | but makes sense to not put the source node allocations on the migration record for an evacuation since the source node should be dead | |
| 21:22:53 | dansmith | and you're never going back there.. no rollback | |
| 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 | dansmith | can't we just stop doing the doubling across the board? | |
| 21:29:43 | mriedem | unlike live migrate, cold migrate/resize | |
| 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 | |