Earlier  
Posted Nick Remark
#openstack-nova - 2018-05-17
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
17:19:48 mriedem has nothing to do with cells v2
17:21:00 melwitt yeah, I was thinking maybe that upcall was disabled by default because of cells v2. but it's not
17:21:16 dansmith it just won't ever make it to the scheduler
17:21:25 mriedem if you're doing split MQ
17:21:41 mriedem i'd be kind of surprised if someone was following that setup if they didn't know what they were doing
17:21:44 melwitt ohh...
17:24:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Metadata-API fails to retrieve avz for instances created before Pike https://review.openstack.org/567878
17:32:17 dansmith edleafe: hmm, so I'm a bit confused actually
17:45:16 edleafe dansmith: do tell
17:46:12 dansmith edleafe: something is off by one, one way or the other and I'm not sure I get it
17:46:24 dansmith either the test is wrong or needs some other tweak, or
17:46:26 dansmith something
17:46:39 edleafe I have a few minutes - let me look
17:46:41 dansmith edleafe: anyway, I've been working on it in parallel to other things and I think I've confused myself out of the way
17:46:50 dansmith so yeah, maybe you should look without me telling you what I'm confused about :)
17:47:15 edleafe I'd much rather confuse myself :)
17:56:05 edleafe dansmith: Looks like this needs to be adjusted, too: https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L367
17:56:22 dansmith edleafe: yeah, I did that
17:56:37 dansmith and I can make the tests pass at that point,
17:56:49 dansmith but ...isn't that just subtracting one from A and adding it to B?
17:56:58 edleafe nope
17:57:06 dansmith well, assuming you mean changing >= to >
17:57:27 edleafe before your change, "num_alts" really was the total returned
17:57:36 edleafe not the number of alternates
17:58:06 edleafe change that line to: >= num_alts + 1
17:58:27 edleafe the '1' is the 'selected' in 'selected_plus_alts'
17:59:17 dansmith ah, heh, that actually makes it click
17:59:23 dansmith changing it to >, which is what I did felt wrong
17:59:36 dansmith but leaving >= and putting the +1 makes it logically make sense to me
17:59:43 edleafe yeah
18:00:09 edleafe I'm re-running the unit tests
18:00:23 edleafe I can push that assuming that the tests pass
18:00:57 dansmith alright
18:02:21 mriedem melwitt: +2 on lyarwood's nova-lvm fix https://review.openstack.org/#/c/569062/
18:07:22 edleafe dansmith: should that patch still be a WIP?
18:07:32 dansmith edleafe: nah I just WIP'd it because no tests
18:08:09 edleafe k
18:08:14 openstackgerrit Matt Riedemann proposed openstack/nova master: Fakelibvirt migrateToURI3 should provide args according to libvirt doc https://review.openstack.org/569047
18:08:58 openstackgerrit Ed Leafe proposed openstack/nova master: Fix interpretation of max_attempts for scheduling alternates https://review.openstack.org/569127
18:09:01 edleafe dansmith: Done ^^
18:09:59 melwitt mriedem: you don't want to wait for lyarwood's test additions?
18:10:10 dansmith edleafe: cool
18:10:54 mriedem unit test?
18:10:59 mriedem was he going to add one?
18:11:14 melwitt on the review he said "nova-lvm is looking good so I'll respin with tests later today."
18:11:19 mriedem yeah, ok
18:11:36 edleafe mriedem: the tests already covered that
18:11:50 edleafe that's why they broke when he made the change
18:12:11 dansmith edleafe: well, they did, but we probably need one to make sure that we're doing the different thing
18:12:18 dansmith I'm not sure where that should go really though
18:13:06 mriedem edleafe: ?
18:13:21 mriedem edleafe: the lvm thing?

Earlier   Later