| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-17 | |||
| 16:54:34 | mriedem | bauzas: meh? | |
| 16:54:40 | dansmith | belmorei_: but it should have been doing that before in the same way in the cell scheduler I mean | |
| 16:54:41 | bauzas | mriedem: yeah, meh. | |
| 16:54:46 | mriedem | bauzas: see channel topic | |
| 16:54:50 | bauzas | mriedem: hahah | |
| 16:54:53 | mriedem | love my idea or get out | |
| 16:54:54 | dansmith | mriedem: nice | |
| 16:54:56 | bauzas | anyway, you're right | |
| 16:55:11 | bauzas | that's not a dev call :p | |
| 16:55:31 | bauzas | mriedem: FWIW, you were confused with the status of libvirt vGPUs | |
| 16:55:45 | bauzas | mriedem: the branch is up for reviews, and I just updated the spec | |
| 16:55:54 | dansmith | belmorei_: maybe it was taking just as long before in the cell, but it wasn't as obvious because it was spread out? | |
| 16:55:59 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM snapshot cleanup https://review.openstack.org/568988 | |
| 16:56:01 | melwitt | dansmith: what do you mean by "cell scheduler"? | |
| 16:56:05 | dansmith | belmorei_: or did you have track_instance_changes enabled? I wouldn't think affinity would work with cellsv1 so I would have assumed no | |
| 16:56:11 | dansmith | melwitt: in cellsv1 there is a scheduler in the cell | |
| 16:56:11 | bauzas | mriedem: jay had some concerns with traits, but I think we should do that in a separate spec | |
| 16:56:20 | belmorei_ | dansmith: maybe | |
| 16:56:26 | belmorei_ | dansmith: track_instance_changes is enabled | |
| 16:56:34 | melwitt | okay. well, with cells v1, wouldn't compute be reporting updates to the scheduler and then it wouldn't build that dict? | |
| 16:56:42 | mriedem | belmorei_: was it disabled when you were doing cells v1? | |
| 16:56:50 | dansmith | melwitt: that's what I just said above | |
| 16:56:59 | bauzas | melwitt: dansmith: catching up the convo, what's the problem with computes updating the scheduler ? | |
| 16:57:05 | dansmith | but I thought we still had to hit the db once per host even with that enbabled | |
| 16:57:09 | bauzas | there is a fanout upcall | |
| 16:57:10 | melwitt | ? so doesn't that make it obvious why it was fast with cells v1 and slow with cells v2? | |
| 16:57:31 | melwitt | because with cells v2 it's building the dict every time in the absence of updates? I'm confused | |
| 16:57:32 | dansmith | melwitt: I had asked him if they had it enabled | |
| 16:57:46 | dansmith | melwitt: affinity doesn't work with cellsv1 so I would have expected they didn't have that enabled | |
| 16:57:50 | dansmith | (afaik) | |
| 16:58:03 | melwitt | okay, I see | |
| 16:58:09 | dansmith | and, I thought we still hit the db once per host even with it enabled for the compute info, but not for the instance info part | |
| 16:58:15 | dansmith | so I wouldn't expect a lage difference | |
| 16:58:16 | melwitt | it probably works for them by way of patches, maybe | |
| 16:58:17 | dansmith | *large | |
| 16:58:23 | dansmith | well, then they're cheating :) | |
| 16:59:05 | melwitt | what's the compute info part? | |
| 17:00:01 | melwitt | the normal host states stuff? | |
| 17:00:13 | dansmith | yeah | |
| 17:01:07 | belmorei_ | ok, we will continue to dig on this | |
| 17:01:13 | belmorei_ | thanks | |
| 17:01:29 | belmorei_ | I'm leaving now | |
| 17:01:59 | mriedem | random thinking out loud, i think the instance info list only uses the uuid from the instances for the affinity filters, | |
| 17:02:07 | melwitt | belmorei_: seeya next week | |
| 17:02:15 | mriedem | we could probably use placement to just pull uuids of consumers that have allocations on a given node and replace that | |
| 17:02:29 | mriedem | would be a lot fast | |
| 17:02:31 | mriedem | *faster | |
| 17:02:39 | dansmith | well, it depends on which piece of his linked function is taking a while | |
| 17:02:46 | dansmith | we do hit the db for the hosts every time, | |
| 17:02:55 | dansmith | but the instance list is the thing we would skip with track_changes | |
| 17:02:59 | dansmith | so yeah, maybe it's just that | |
| 17:03:09 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L753 could also avoid some default joins... | |
| 17:03:16 | dansmith | still once call to placement per instance would be not great | |
| 17:03:24 | dansmith | er per compute | |
| 17:03:27 | mriedem | i meant per compute | |
| 17:03:43 | mriedem | right now it looks like this does a full instance list per host including default joined tables | |
| 17:03:50 | mriedem | which is i think info_cache and security_groups | |
| 17:03:55 | mriedem | and all the affinity filters care about is the uuid | |
| 17:03:59 | dansmith | yeah probably | |
| 17:04:04 | melwitt | good find | |
| 17:04:13 | mriedem | https://blueprints.launchpad.net/nova/+spec/put-host-manager-instance-info-on-a-diet | |
| 17:04:19 | mriedem | maybe i should have worked on this blueprint some more | |
| 17:05:16 | dansmith | it would be even better to just have a single db call that would take the compute uuids, and return a dict of {host: [uuid, ]} | |
| 17:05:48 | dansmith | although it does have a full instance object in the result there.. are we sure that's not used? | |
| 17:05:59 | mriedem | pretty sure, read that blueprint | |
| 17:06:20 | mriedem | it used to also pull the instance.instance_type_id for the TypeAffinityFilter, but we removed that filter | |
| 17:06:26 | mriedem | s/pull/use/ | |
| 17:06:39 | dansmith | yeah, but any filter out there that expects a full list of instances in the host state will care | |
| 17:06:46 | mriedem | sure, but. | |
| 17:06:50 | mriedem | f'em | |
| 17:07:11 | dansmith | we could put fake things in there that would lazy-load if they touch them :) | |
| 17:08:02 | dansmith | but okay yeah, that bp text makes sense | |
| 17:08:07 | mriedem | so if you have a filter that cares about the network info cache, and we stop auto-joining on that by default, you'd start lazy-loading and see a perf hit during scheduling | |
| 17:08:11 | dansmith | (he says noting he's the approver) | |
| 17:08:20 | dansmith | yeah | |
| 17:08:36 | dansmith | makes it look like their fault though! :) | |
| 17:08:40 | mriedem | but at that point, you either (1) live with it (2) fork the host manager code (3) upstream your filter | |
| 17:09:16 | dansmith | it'd be nice to get some clarification from them that this is indeed where their hotspot is (grabbing that large instance list) | |
| 17:10:38 | mriedem | yeah and they are both gone now | |
| 17:10:41 | mriedem | tired of our antics | |
| 17:11:00 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update instance.availability_zone during live migration https://review.openstack.org/567701 | |
| 17:14:46 | melwitt | did y'all look at the bug I linked earlier? https://bugs.launchpad.net/nova/+bug/1737465 | |
| 17:14:47 | openstack | Launchpad bug 1737465 in OpenStack Compute (nova) "[cellv2] the performance issue of cellv2 when creating 500 instances concurrently" [Undecided,Confirmed] - Assigned to Jiang (jiangpf) | |
| 17:14:58 | dansmith | yeah | |
| 17:15:52 | dansmith | I'm sure it's the same thing, but I can't really make sense of most of the text in there | |
| 17:16:00 | melwitt | okay, I was thinking they're hitting the same thing in _get_instance_info and that's doing the instance list, it doesn't do anything else | |
| 17:16:41 | melwitt | yeah, it's not easy to read but they did point out it was the call to _get_instance_info where they get the slowdown | |
| 17:17:12 | dansmith | yeah, that's why I say I'm sure it's the same | |
| 17:17:29 | mriedem | "Because cellv2 have disable instance info cache." - i assume that means they disabled track_instance_changes | |
| 17:17:55 | melwitt | yeah, me too. so you're thinking there could possibly be something else in _get_instance_info that could cause a slowdown other than the instance list, when you said it would be good if they could clarify? | |
| 17:18:24 | dansmith | no, I'm just saying it would be good to clarify, since he linked at a rather high-up spot | |
| 17:18:29 | melwitt | mriedem: I think it means because the upcall is disallowed by default, yeah? | |
| 17:18:39 | melwitt | dansmith: oh, okay | |
| 17:18:45 | mriedem | melwitt: no | |
| 17:18:58 | mriedem | track_instance_changes is on by default, | |
| 17:19:07 | mriedem | if he's using devstack, then yes that upcall is disabled | |
| 17:19:21 | mriedem | https://docs.openstack.org/nova/latest/configuration/config.html#workarounds.disable_group_policy_check_upcall is not disabled by default though | |
| 17:19:30 | melwitt | but with cells v2 it won't cache it, because the upcall is disabled by default, right? oh nevermind | |
| 17:19:34 | mriedem | so out of the box brand new install that's not devstack is going to be doing the rpc blast upcall | |
| 17:19:37 | mriedem | no | |