Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-08-08
17:52:36 jaypipes dansmith: are you saying we *currently* delete the allocation from the API?
17:52:51 dansmith no I'm saying we should do that, and that's what the bug is about
17:53:04 dansmith mriedem: right?
17:54:00 jaypipes dansmith: if an ocata api local-deleted an instance, then is upgraded to pike, the compute host is upgraded to pike as well, wouldn't there be an allocation left over for the compute host that would need deleting?
17:54:36 mriedem melwitt: question in https://review.openstack.org/#/c/470578/
17:54:52 dansmith jaypipes: sure, but you're doing that right?
17:54:58 melwitt mriedem: I'll add console proxy stuff to the cells docs. I was thinking to mention that "in the future" we're planning to change the location, so people have a heads up
17:55:10 mriedem dansmith: correct
17:55:30 mriedem during local delete in the API, we delete shit from external services like cinder/neutron because the compute is down or the instance doesn't have a host (it's offloaded)
17:55:30 melwitt mriedem: also thanks for rebasing those backport series. didn't even get a chance to ask you yet :)
17:55:37 mriedem placement would be included in "external shit"
17:55:42 jaypipes dansmith: heh, yes, I am. sorry, I thought you were saying there'd be no need for that code.
17:56:22 dansmith jaypipes: no, I don't think there's a need for the shelve offload part I commented on, but for local delete we should hope to never hit this code if we deleted from the api as expected
17:56:30 jaypipes ahhh, sorry.
17:57:08 dansmith jaypipes: if there is an allocation for us that refers to an instance that is marked as deleted, then we should delete the allocation
17:57:19 mriedem when we shelve offload an instance, the allocations for that compute node should be cleaned up by the RT
17:57:24 dansmith jaypipes: note my comment about notfound for later though
17:57:26 melwitt mriedem: to your question, yeah it seems like it would be racy. not sure what else to do though.
17:57:43 dansmith mriedem: we clean the allocations before we offload it, so I don't think we need to handle cleanup there
17:57:58 mriedem dansmith: via RT yeah?
17:58:03 dansmith mriedem: otherwise we couldn't have been offloaded
17:58:24 mriedem melwitt: you could call the update_available_resource method directly
17:58:26 dansmith mriedem: yeah we call direct to self._update or wahtever
17:58:32 mriedem self.service.manager.update_available_resource or whatever
17:58:41 melwitt oh, okay
17:58:44 dansmith mriedem: yeah, I assumed you mean the periodic osrry
17:58:57 mriedem https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4345
17:59:12 mriedem right it calls rt.update_usages
17:59:14 mriedem *usage
17:59:22 mriedem and the offloaded state is in the ALLOW_REMOVAL_STATES or whatever
17:59:45 mriedem so the only case for local delete of allocatoins in the api is when the instance isn't shelved offloaded but the compute service is down
18:10:23 mriedem jaypipes: tbc, i dont know that we need to handle the local delete in the api thing right now
18:10:52 mriedem unless the computes won't cleanup those allocations when they come back up
18:11:55 cdent I’d like it if we could merge some of the pending code as we have several in flight bug fixes, all vaguely interrelated, and it is hard to find yet more bugs while those are still in the way (and not all in the same stack). We may just need to be willing to have some bugs in master (we already do) so we can find and fix more of them.
18:13:37 dansmith mriedem: jaypipes: agreed
18:15:46 jaypipes dansmith, mriedem: so, bottom line, the changes in https://review.openstack.org/#/c/491850/ are fundamentally OK aside from some of the typos dansmith noted?
18:18:18 mriedem i'll have to go through it,
18:18:24 mriedem i've been waiting for dansmith to be ok with things
18:22:03 mriedem i need to push up something quick and then i'll take a look
18:25:40 dansmith jaypipes: not just typos right?
18:25:52 dansmith or are you going to leave the shelve_offloaded thing?
18:26:09 jaypipes dansmith: I can remove that, sure.
18:26:34 dansmith I think we should remove it if we can't explain it and deal with it later if we determine there's a gap there
18:26:42 dansmith it'll be a shelve gap, of which there are many
18:26:59 dansmith jaypipes: I say clean it up and let mriedem take a look fresh
18:27:09 dansmith not like we're going to get check results any time soon anyway
18:27:18 jaypipes kk
18:27:36 melwitt this is interesting. I'm running the test_boot_from_volume func test after rebasing and it used to allow an in-place resize (1 vcpu to 1 vcpu on a host with only 1 vcpu) but now it's saying it needs 2 vcpus to do the resize
18:28:33 mriedem melwitt: yup :)
18:28:41 mriedem we double the allocations in the scheduler
18:28:50 mriedem well, sum rather than take the max
18:29:11 mriedem https://review.openstack.org/#/c/490085/
18:29:38 melwitt yeah, I remember seeing some mention of that in the channel chats. I guess I'm surprised at the behavior change? I didn't think we'd want to change how that works because users will hit this
18:29:59 melwitt well, I guess maybe resize on same host isn't too common so maybe they won't hit it
18:30:58 mriedem dansmith and i talked about why sum vs max() in https://review.openstack.org/#/c/490085/ but now i can't really remember the reasoning
18:31:17 mriedem i think to basically be consistent with multiple hosts
18:31:51 mriedem so if you have an allocation for host A and host B, then the instance allocations are 1 VCPU on each
18:31:53 dansmith it's hypervisor-dependent behavior
18:32:00 dansmith so the only sane thing we can do is double-claim everything
18:32:30 melwitt yeah, I mean I could see the logic in it. I think my concern is with the change in behavior. that's something to call out in the release notes/docs I think
18:32:54 dansmith it's not,
18:32:54 melwitt i.e. thing that used to work, no longer works
18:33:00 dansmith because it's a scheduler thing users won't see
18:33:14 dansmith i.e. they can't ask for resize to the same host
18:33:30 dansmith I mean, maybe it's worth calling out for operators that things you think used to fit one way won't anymore,
18:33:32 dansmith if that's what you mean
18:33:43 melwitt it is in my test. I have a compute node with 1 vcpu and was doing a resize on it, 1 vcpu to 1 vcpu. that used to work, now it doesn't because it wants 2 vcpus and that violates the compute node resource constraints
18:33:46 dansmith but it's not like fundamentally different externally visible behavior
18:35:18 mriedem dansmith: can you explain the hypervisor-dependent behavior? not sure i'm following you there.
18:35:45 dansmith mriedem: meaning libvirt very loosely defines a vcpu and thus if a thing isn't running it's not really using any resource,
18:35:53 melwitt yeah, since we don't default allow_resize_same_host=True then I think it's less likely users will see it externally. so maybe more realistically it'll come up as you said for operators that are setting allow_resize_to_same_host=True for testing etc
18:35:56 dansmith but if you pin cpus to dedicated mode then it will
18:36:28 dansmith melwitt: that flag is really just for tempest and the gate yeah
18:37:53 dansmith a general prelude about placement and resource accounting differences is useful, which I think bauzas has up right?
18:38:18 dansmith because there are differences between the old filters and placement's logic, some of which we rolled over with ocata
18:39:57 melwitt yeah, that would be really good to have
18:40:00 mriedem the prelude reno doesn't go into those details
18:40:09 mriedem https://review.openstack.org/#/c/491424/
18:40:28 melwitt maybe a pointer to a doc if too much detail would be big
18:40:35 mriedem it's not in a doc :)
18:40:41 melwitt when it's in a doc
18:40:58 mriedem when it's in a doc would have to be before we release 16.0.0
18:41:17 mriedem we could put something here https://docs.openstack.org/nova/latest/user/placement.html#pike-16-0-0
18:41:21 mriedem and have the prelude point at that
18:41:29 dansmith mriedem: it's totally in there
18:41:43 dansmith mriedem: it says "this is not an exhaustive list" -> "and other stuff, kthx"
18:41:46 melwitt yeah. I dunno, just imagining people noticing this stuff and wondering who the what now
18:42:02 mriedem dansmith: you're joking right?
18:42:07 dansmith mriedem: yes
18:42:08 melwitt though it won't be for like a year since that's when people would start trying Pike
18:42:40 mriedem jaypipes was talking about putting up a devref for how resize is going to be handled
18:42:44 mriedem but maybe for now,
18:42:58 mriedem we go cheap and easy and throw a bullet in https://docs.openstack.org/nova/latest/user/placement.html#pike-16-0-0 and then reference that from the prelude for 'more info' type stuff for now
18:43:37 mriedem i.e. during scheduling, we get the allocation candidates from placement, and then use those to get the compute nodes from the cells, and iterate the results using the enabled filters
18:44:14 mriedem then we iterate the hosts and make allocation requests for the instance against a given host, retrying as necessary until an allocation is made or all allocations are exhausted, which results in NoValidHost
18:44:31 mriedem for a move operation, allocations are made on the source and dest hosts,
18:44:39 mriedem for a resize to the same host, allocations are summed on the same host
18:44:49 mriedem ^ is that sufficient for a note in https://docs.openstack.org/nova/latest/user/placement.html#pike-16-0-0 ?
18:44:50 melwitt we can just copy-paste this into the doc :)
18:45:33 mriedem i'll just throw something up using that and we can take a look

Earlier   Later