| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-17 | |||
| 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? | |
| 18:13:40 | dansmith | mriedem: oh we thought you were talking about the alternates thing | |
| 18:14:32 | dansmith | edleafe: we probably just need to validate that max_attempts=1 gets you 1 result | |
| 18:14:55 | mriedem | no i don't talk about that | |
| 18:22:15 | edleafe | dansmith: does this test satisfy? https://github.com/openstack/nova/blob/master/nova/tests/unit/scheduler/test_filter_scheduler.py#L915-L917 | |
| 18:23:52 | dansmith | edleafe: no, because the essential change is that max_attempts=1 should end up with zero alternates to filter | |
| 18:25:55 | edleafe | ok, let me add that | |
| 18:38:37 | openstackgerrit | Ed Leafe proposed openstack/nova master: Fix interpretation of max_attempts for scheduling alternates https://review.openstack.org/569127 | |
| 18:38:52 | edleafe | dansmith: ^^ now with shiny new unit test | |
| 18:42:40 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Avoid unnecessary joins in HostManager._get_instances_by_host https://review.openstack.org/569218 | |
| 18:44:00 | dansmith | edleafe: I forgot about how that one test has num_alternates=0 | |
| 18:44:06 | dansmith | which is kiiinda the same thing | |
| 18:44:26 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Avoid unnecessary joins in HostManager._get_instances_by_host https://review.openstack.org/569218 | |
| 18:44:38 | mriedem | dansmith: melwitt: ^ the small optimization noted earlier | |
| 18:55:21 | dansmith | edleafe: you know, actually I think what we probably want is to mock out the interface between _schedule() and _get_alternate_hosts() and make sure that num_alts out and in from those (respectively) includes the new offset | |
| 18:55:39 | dansmith | edleafe: because testing them together just hides that we made a change, which is why that new unit test is kinda forced | |
| 18:55:48 | dansmith | edleafe: I'm happy to do that if you want | |
| 18:55:58 | dansmith | ...if you agree | |
| 18:57:02 | melwitt | mriedem: ack | |
| 19:06:26 | edleafe | dansmith: go for it. I have meetings for the next two hours | |
| 19:07:37 | edleafe | dansmith: and looking at the code, it *was* working correctly, just poorly named. If we had simply changed 'num_alts' to 'num_returned', it would have continued to work, and made more sense to someone reading it | |
| 19:22:08 | openstackgerrit | Eric Fried proposed openstack/nova master: Normalize inventory from update_provider_tree https://review.openstack.org/569132 | |
| 19:22:09 | openstackgerrit | Eric Fried proposed openstack/nova master: libvirt: get_inventory => update_provider_tree https://review.openstack.org/560444 | |
| 19:22:10 | openstackgerrit | Eric Fried proposed openstack/nova master: libvirt: Don't report DISK_GB if sharing https://review.openstack.org/560459 | |
| 19:22:11 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.has_inventory_changed for new fields https://review.openstack.org/569228 | |
| 19:22:29 | efried | edleafe, cdent, dansmith: y'all may want to take a look at these ^ | |
| 19:22:51 | efried | the bottom two anyway (which are intended to fix tempest on the top two) | |
| 19:24:14 | efried | bhagyashris_: FYI, hoping those bottom two patches clear up the tempest failures. | |
| 19:24:53 | efried | bhagyashris_: We were running out of hosts in tempest because we were no longer setting allocation ratios. I imagine the 16.0 allocation ratio on VCPU was what was killing us. | |
| 19:33:41 | cdent | seems sane, efried, left a query on the second one | |
| 19:34:36 | efried | cdent: Thanks. Seems like we're reworking the allocation ratio thing - but I think it still is (or can be) driven from conf and/or via the compute node record, and the virt still doesn't necessarily have access to the information. | |
| 19:34:58 | efried | cdent: I really wanted to catch Jay to get the skinny on that, but he seems to be out today. | |
| 19:35:53 | efried | cdent: As it stands, if the virt driver thinks it knows better, it can set those and _normalize... won't overwrite them. | |
| 19:35:55 | cdent | Yeah, that was the source of my confusion: given _now_, what you've got seems right | |
| 19:36:20 | efried | Glad we agree on that. | |
| 19:36:45 | efried | And I don't know what all the future is supposed to hold for this. So _now_ is about all I can account for... | |
| 19:37:22 | cdent | Indeed | |
| 19:38:19 | efried | See you in 'couver. | |
| 19:39:33 | efried | mriedem: I caught up with Gerald. He cited a customer scenario with 500 volumes on an instance. So 1024 seems like a safe and relatively future-proof number. | |
| 19:52:07 | dansmith | efried: um, I had already provided a customer scenario with 600, although no other detail, so clearly 1024 makes sense in a real-world situation | |
| 19:52:49 | efried | dansmith: Okay, I didn't follow that conversation to its end. Just got an answer so thought I would post it. | |
| 19:53:50 | dansmith | efried: how is "some customer wants to use 500" a reasonable argument? I guess my overt sarcasm wasn't overt enough | |
| 19:54:50 | efried | now you've lost me. | |
| 19:55:26 | dansmith | efried: I want to hear the *reason* for wanting to use 500 volumes on a single guest, not just that someone thinks it's a good idea | |
| 19:55:27 | efried | You mean we should require said customers to give justification that we agree with, as opposed to letting us tell them how many volumes they should be using instead? | |
| 19:55:39 | efried | yeah, got it. | |
| 19:56:18 | dansmith | the bug/bp being filed seems to indicate that someone somewhere wants to do it | |