| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-08 | |||
| 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 | melwitt | i.e. thing that used to work, no longer works | |
| 18:32:54 | dansmith | it's not, | |
| 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 | |
| 18:46:06 | melwitt | ++ | |
| 18:49:52 | jaypipes | dansmith: pls see latest coment on https://review.openstack.org/#/c/491850/1/nova/compute/resource_tracker.py | |
| 18:50:01 | jaypipes | dansmith: line 1060 | |
| 18:50:06 | jaypipes | dansmith: ? for you there. | |
| 18:50:59 | melwitt | guh, looks like it's a PITA to change the resources SmallFakeDriver has | |
| 18:52:24 | mriedem | melwitt: i did something like this, sec | |
| 18:52:36 | mriedem | melwitt: one thing is just using FakeDriver | |
| 18:52:39 | mriedem | it has more resources | |
| 18:52:42 | dansmith | jaypipes: well, you are putting a continue in there that wasn't there before, but also, I think it's actually dead code | |
| 18:52:46 | mriedem | the ServerMovingTests use that | |
| 18:52:48 | mriedem | for the same reason | |
| 18:52:59 | dansmith | jaypipes: L1028 clears the tracked instances, then you roll through and exclude things that are in that set, but it's empty right? | |
| 18:53:08 | melwitt | mriedem: I found some examples but they involve stubbing out the compute driver load | |
| 18:53:47 | jaypipes | dansmith: but the call to _update_usage_from_instance() on line 1042 adds instances back into tracked_instances :) | |
| 18:53:58 | jaypipes | dansmith: perfectly. clear. | |
| 18:54:22 | dansmith | jaypipes: ah right, well then you are changing it | |
| 18:54:37 | jaypipes | dansmith: I know, it's awfulness. | |
| 18:55:36 | mriedem | melwitt: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L1074-L1077 | |
| 18:56:02 | melwitt | mriedem: yesss thank you | |
| 18:56:33 | dansmith | jaypipes: so we only have things in tracked_instances that are not offloaded or deleted, and your assertion is that if we processed them in that thing, added to tracked, and then found an allocation we can ignore entirely | |
| 18:57:07 | jaypipes | dansmith: yup. those represent the "normals" | |
| 18:57:17 | jaypipes | dansmith: and we don't need to remove any allocations for them. | |
| 18:57:25 | jaypipes | dansmith: they're active, stopped, paused, etc | |
| 18:57:43 | jaypipes | dansmith: still consuming resources on the node and that's acceptable. | |
| 18:58:03 | dansmith | jaypipes: yeah okay re-reading my scenario, I see that update would have excluded anything we in those states _and_ things we don't have running on our host I guess, the latter being the critical point | |
| 18:58:17 | jaypipes | right. I was just adding some log statements in there... | |
| 18:59:16 | dansmith | I'm not sure it's less confusing the way you have it written, but don't change it now | |
| 18:59:29 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add track_instance_changes note in disable_group_policy_check_upcall https://review.openstack.org/490627 | |
| 18:59:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: considerations before deploying multiple cells https://review.openstack.org/491885 | |
| 19:00:50 | cdent | melwitt: you might take a look at gibi’s tests where he’s messing with resize to same host. that’s where a lot of the fiddling and discussion about make tests happy has happened: around ps4 on https://review.openstack.org/#/c/491529/ | |
| 19:00:51 | mriedem | dansmith: posed a question in my own patch ^ for https://review.openstack.org/#/c/491885/ to how we'd best like to communicate the online data migrations not being multi-cell aware | |
| 19:01:26 | melwitt | thanks cdent | |
| 19:02:04 | dansmith | mriedem: I was thinking about this after you brought it up, but people can't have multiple cells worth of things needing migration, so I'm not sure we need to even say anything, right? | |
| 19:02:35 | mriedem | dansmith: because a new cell would be empty and we couldn't fallback to it anyway? | |