Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-09
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
21:36:55 dansmith and I think that's for the "if host changed" case,
21:36:56 dansmith where we can either fall through or bail
21:37:12 dansmith I think we might be over-obsessing here
21:37:16 mriedem yeah if the host changes we're here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1816
21:37:26 mriedem and then it falls through to the normal delete path
21:37:36 dansmith I'm not sure whether notifications consumers will do something useful with the soft/hard indication
21:37:54 mriedem i agree this is probably not the end of the world
21:38:23 dansmith the missing notifications, and the quota bug behind this are definitely more important
21:38:37 dansmith gibi might have a better handle on how much this does or does not matter
21:39:27 melwitt I was thinking my main concern would be whether the start/end notifications match, and also that they match the request that was issued from the user
21:39:45 melwitt for the first, I saw the context manager makes sure the start/end match
21:39:47 dansmith the context manager makes sure they match I think
21:40:45 melwitt for the last, I guess that's where the delete_type comes in. I was just trying to think, how someone might use this and if they're listening to notifications, they may also be looking at what requests were issued and correlating notifications with those
21:41:43 melwitt to get like a bundle of "what happened" which would be 1) user issued delete_type X 2) nova emitted delete_type X start 3) nova emitted delete_type X end
21:42:55 dansmith I'm not sure what the point of correlating the requests and the notifications really is, but we don't really know what (all) people use these for
21:42:59 melwitt if they're doing log collecting and correlating. anyway, that's the thing I could think of it mattering
21:43:25 dansmith mriedem: I guess my point is, this is demonstrably better than what we have now, I think, especially considering the quota thing behind it
21:43:46 dansmith in some very teensy windows of a less-mainstream feature, you might get a delete type you didn't expect
21:43:49 melwitt at my last company, use used the old notification driver that emits notifications to the logs. and we had those in splunk and correlated things like that, to look at when people opened bugs etc
21:45:00 melwitt I think what I'm saying is what the patch is doing, so it makes sense to me to keep everything saying "soft delete" if the user issued "soft delete"
21:45:57 dansmith well, it's not in the first case he brought up
21:46:05 mriedem the user doesn't technically request "soft" delete
21:46:11 mriedem it's all driven by config
21:46:17 mriedem it's totally not interoperable
21:46:55 melwitt erg, right. notifications probably have the request-id too right? so that's what anyone would use to correlate anyway
21:47:02 dansmith yeah
21:47:40 melwitt okay, that addresses what I was thinking
21:48:42 mriedem so i think i might still pull this down, add some comments, and add a test for the soft-delete case where there isn't a host but it's in a cell (any cell) and the build request was already gone
21:49:07 mriedem because i'm pretty sure i'm right here https://review.openstack.org/#/c/437222/7/nova/tests/unit/utils.py
21:49:17 mriedem then i'll rebase my stuff on top of it
21:49:23 mriedem and then it's pants off dance off
21:53:05 dansmith so, wait,
21:53:23 dansmith we notify delete.started..delete.ended in the api, or soft_delete.started..ended,
21:53:34 dansmith and then again in compute in both cases?

Earlier   Later