| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 21:03:23 | dansmith | the context calls | |
| 21:03:23 | mriedem | oh | |
| 21:03:25 | mriedem | yeah, well, | |
| 21:03:32 | mriedem | see ^ for the last 3 hours :) | |
| 21:03:37 | dansmith | heh yeah I know | |
| 21:05:41 | mriedem | dansmith: comments in https://review.openstack.org/#/c/437222/ | |
| 21:05:45 | mriedem | the notification one | |
| 21:06:50 | cburgess | mriedem So https://review.openstack.org/#/c/417541 should be abandoned. I'll have yuming do that shortly. Our work is now covered by johnthetubaguy work in https://review.openstack.org/#/c/373203/ | |
| 21:07:12 | mriedem | cburgess: i can abandon it if you want | |
| 21:07:17 | cburgess | Go ahead | |
| 21:07:35 | cburgess | mriedem Abandon with great abandon. | |
| 21:07:49 | dansmith | mriedem: so in the delete_while_booting, we can't soft delete, so it's always going to be a hard instance-is-gone | |
| 21:08:03 | dansmith | because we're just deleting the mapping | |
| 21:08:14 | dansmith | well, wait | |
| 21:08:55 | dansmith | yeah, right, | |
| 21:09:03 | dansmith | because if we deleted that, then we're going to end up with no instance record when the conductor realizes that we won | |
| 21:09:07 | dansmith | L1712 | |
| 21:10:57 | dansmith | well, wait, | |
| 21:11:18 | mriedem | it seems like we just don't support soft_delete for the case that the instance isn't in a cell yet | |
| 21:11:19 | dansmith | yeah | |
| 21:11:28 | dansmith | right | |
| 21:11:38 | mriedem | soft_delete is also totally dependent on the compute deleting the instance | |
| 21:11:44 | mriedem | and if it never landed in a cell, there is no compute to clean it up | |
| 21:12:06 | mriedem | seems like a good comment to add in the code as a reminder of that | |
| 21:12:33 | mriedem | we could do that in a follow up, like my ask for a comment about why we avoid the lazy-load in the context manager for sys meta | |
| 21:12:45 | mriedem | cburgess: done, and the bp in lp too | |
| 21:13:28 | cburgess | cool | |
| 21:13:30 | mriedem | this patch still doesn't have a test to make sure that the soft_delete case is handled when the instance is in cell0 | |
| 21:13:33 | dansmith | mriedem: yeah | |
| 21:13:55 | dansmith | mriedem: I can do that, or you can throw it in while you're stacking your others on top here | |
| 21:14:14 | mriedem | let me think about what i just said, | |
| 21:14:19 | dansmith | yeah, that... | |
| 21:14:22 | dansmith | would be a problem | |
| 21:14:27 | mriedem | if it's in cell0, again, it's we can't soft delete it | |
| 21:14:29 | dansmith | but there's also no resources to clean up | |
| 21:15:19 | mriedem | https://review.openstack.org/#/c/437222/7/nova/compute/api.py@2126 | |
| 21:16:39 | dansmith | that cast is just going to go into the ether | |
| 21:16:59 | dansmith | if you delete it again, it hard deletes right? | |
| 21:18:03 | openstackgerrit | Sarafraj Singh proposed openstack/nova-specs master: WIP: Replace live migration steps and delays with memory iteration logic https://review.openstack.org/443339 | |
| 21:20:25 | dansmith | mriedem: from the api perspective, what we have _might_ make the most sense | |
| 21:20:36 | dansmith | you soft delete, it stays around half deleted, | |
| 21:20:56 | dansmith | but never gets culled because there is no compute for it | |
| 21:20:57 | dansmith | then you hard delete it to make it go away | |
| 21:20:58 | dansmith | on the other hand, | |
| 21:21:09 | mriedem | you mean force-delete? | |
| 21:21:28 | dansmith | just making it kill the instance immediately if it's in cell0 also makes sense, since there's no reason to keep a dead instance around | |
| 21:21:40 | dansmith | soft-delete being for "oops, I deleted a thing that I wanted to keep" | |
| 21:21:57 | mriedem | i don't think force-delete would fix anytihng here | |
| 21:22:04 | mriedem | but i'm not following the half-deleted case | |
| 21:22:04 | openstackgerrit | Sarafraj Singh proposed openstack/nova-specs master: Spec to add per instance timeout for LM https://review.openstack.org/442809 | |
| 21:22:09 | mriedem | i put some comments in there if you want to read those and reply | |
| 21:22:27 | dansmith | mriedem: https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L782-L785 | |
| 21:22:40 | mriedem | basically i think the only issue now is the notification is lying if it says we do a soft-delete but we actually hard-delete in cell0 | |
| 21:23:07 | mriedem | what triggers that InstanceInvalidState though? | |
| 21:23:18 | dansmith | mriedem: lies how? because it doesn't call it a soft delete? | |
| 21:23:22 | openstackgerrit | Sarafraj Singh proposed openstack/nova-specs master: Add live-migration-force-after-timeout spec https://review.openstack.org/438467 | |
| 21:23:27 | mriedem | must_have_launched=True | |
| 21:23:31 | dansmith | mriedem: must_have_launched | |
| 21:23:32 | dansmith | yeah | |
| 21:24:10 | mriedem | what sets launched_at? compute? | |
| 21:24:24 | mriedem | yup _update_instance_after_spawn | |
| 21:24:26 | dansmith | yeah | |
| 21:25:18 | mriedem | see https://review.openstack.org/#/c/437222/7/nova/compute/api.py@1794 | |
| 21:25:20 | mriedem | for the lie | |
| 21:26:01 | dansmith | oh, calls it a soft delete when it's not | |
| 21:26:04 | mriedem | yeah | |
| 21:26:18 | mriedem | i mean that's very minor | |
| 21:26:23 | mriedem | but i think it's technically wrong | |
| 21:26:55 | dansmith | if we do that, | |
| 21:26:56 | dansmith | hmm | |
| 21:27:07 | dansmith | you can get there if you have a non-cell0 cell too right? | |
| 21:27:22 | mriedem | but no host? | |
| 21:27:28 | dansmith | if you race with compute owning the instance | |
| 21:27:35 | dansmith | the instance_mapping has a cell, | |
| 21:27:38 | dansmith | because scheduler decided, | |
| 21:27:44 | dansmith | but compute hasn't instance.host=$me yet | |
| 21:29:34 | mriedem | what sets instance.host in the compute? | |
| 21:29:38 | mriedem | for spawn i mean, i'm not seeing that | |
| 21:29:41 | dansmith | yeah, hang on | |
| 21:30:10 | dansmith | I always have to dig for this I dunno why | |
| 21:31:31 | melwitt | it's in instance_claim in the RT | |
| 21:31:40 | dansmith | oh, that's probably why | |
| 21:31:56 | mriedem | i figured that gd low down RT was involved | |
| 21:31:59 | dansmith | https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L336-L344 | |
| 21:32:12 | melwitt | haha, yeah. it's hidden there, for fun! | |
| 21:32:46 | mriedem | RT is like a leprachaun | |
| 21:32:56 | mriedem | full of tricks | |
| 21:33:50 | mriedem | dansmith: ok so back to https://review.openstack.org/#/c/437222/7/nova/compute/api.py@1794 if it's not in cell0 and it doesn't have a host yet, but let's say it's in cell1, | |
| 21:34:00 | mriedem | we still delete the instance | |
| 21:34:27 | mriedem | so i think that just means, when the reclaim task (or whatever it's called) in the compute runs, it just won't see that instance | |
| 21:34:32 | mriedem | and that's fine, it's already gone | |
| 21:34:39 | mriedem | so it was an immediate soft delete :) | |
| 21:34:46 | mriedem | right? | |
| 21:35:17 | dansmith | unless we got here because instance wasn't created in the database | |
| 21:35:28 | dansmith | but I _think_ that means it's gone-gone too | |
| 21:35:37 | mriedem | which database? | |
| 21:35:42 | mriedem | cell db? | |
| 21:36:17 | dansmith | yeah the only place instances get created | |
| 21:36:31 | dansmith | I think we can also fail to delete there if host changes, right? | |
| 21:36:38 | dansmith | we're in a try..except there | |