| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-15 | |||
| 16:03:19 | fried_rice | belmoreira: set_inventory_for_provider et al should be shorting out, not talking to the placement API, for any but the changed node. Are you seeing those hit placement? | |
| 16:03:55 | gibi | so I need to found an example where there is two possible valid mapping one with overlap between groups and one without. If I can make such an example then I need to change the algo | |
| 16:04:02 | fried_rice | belmoreira: Because https://github.com/openstack/nova/blob/d231a420d0d4865bb19da513af416cb8bc89010f/nova/scheduler/client/report.py#L920-L924 | |
| 16:04:25 | fried_rice | gibi: ++. If anyone can do it... | |
| 16:04:52 | jaypipes | gibi, fried_rice: so, thinking through the group_policy=isolate case... as mentioned, we can assume that placement returned allocations of same-resource-class requests against *different* providers so I don't think we need to modify this algorithm to account for group_policy=isolate, because there's no way the algorithm *could* place amounts of the same resource class on the same provider. | |
| 16:05:02 | jaypipes | gibi: sorry :( | |
| 16:05:29 | gibi | jaypipes: don't be sorry, you found a deep question here | |
| 16:05:35 | gibi | jaypipes: so I'm glad | |
| 16:05:53 | jaypipes | gibi: I'm not sure it's actually a scenario we need to be concerned about. :) | |
| 16:05:57 | gibi | jaypipes: it is just murphy's law | |
| 16:06:06 | jaypipes | heh | |
| 16:06:28 | gibi | isolate is global for the whole request, isn't it? | |
| 16:06:34 | fried_rice | yes | |
| 16:06:43 | gibi | so every group will have its own RP in the allocaton | |
| 16:06:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: ironic: complete the flavor data migration started in pike https://review.openstack.org/637217 | |
| 16:06:50 | mriedem | belmoreira: jaypipes: dansmith: edleafe: ^ probably the most detailed commit message ever | |
| 16:06:53 | mriedem | jroll: ^ | |
| 16:07:22 | gibi | and the size of the allocation is always the size of the request | |
| 16:07:35 | gibi | so there is no way that in this allocation an overlapping mapping can be found | |
| 16:08:17 | gibi | because overlap means the size of an allocation is at least the size of two request | |
| 16:08:55 | gibi | so even if a partial solution found with overlap, the remaining RPs cannot fulfill the remaining groups | |
| 16:09:24 | fried_rice | gibi: right, I think you would either end up short an allocation, or short a provider. | |
| 16:09:35 | gibi | fried_rice: yeah, feels like it | |
| 16:09:38 | mriedem | mgoddard: johnthetubaguy: you might also care about https://review.openstack.org/#/c/637217/ | |
| 16:09:49 | fried_rice | so, maybe safe by serendipity | |
| 16:10:11 | leakypipes | gibi: exactly my thoughts. | |
| 16:10:17 | leakypipes | fried_rice: ya | |
| 16:10:51 | mgoddard | mriedem: that looks like something I would care about, adding to review list | |
| 16:10:52 | leakypipes | fried_rice: well, not necessarily serendipity. safe due to placement's alloc cands selection algorithm already working properly :) | |
| 16:11:17 | gibi | leakypipes, fried_rice: OK, I still write up a partial overlap case to see that I'm right | |
| 16:13:25 | gibi | https://etherpad.openstack.org/p/CdbhX84nKI | |
| 16:13:34 | gibi | leakypipes, fried_rice: I think I rest my case now | |
| 16:15:21 | belmoreira | fried_rice: yes, we have those changes. I think is simple the iteration... | |
| 16:18:05 | fried_rice | belmoreira: I'm asking whether you're seeing 1700x3 calls to placement from that loop, or if all the thrashing is happening locally | |
| 16:18:19 | mriedem | tssurya: you can drop your -W on https://review.openstack.org/#/c/635146/ | |
| 16:19:31 | belmoreira | fried_rice: we don't see placement calls in the loop | |
| 16:19:32 | fried_rice | belmoreira: I find it hard to imagine that 5100 local dict compares on the ProviderTree are taking 6h | |
| 16:19:36 | fried_rice | wow | |
| 16:20:13 | leakypipes | gibi: yep, the current algo in the mapping patch does indeed have the possibility of assigning the same RP to different groups erroneously. :( | |
| 16:20:40 | leakypipes | Well, cool, I'm glad my rambling and injection of nonsense into this conversation has resulted in something of actual value. | |
| 16:20:43 | fried_rice | belmoreira: Good thing ProviderTree isn't an ovo :P | |
| 16:20:49 | leakypipes | job done. weekend awaits. | |
| 16:21:59 | belmoreira | fried_rice: is not 5100 compares | |
| 16:22:41 | tssurya | mriedem: thanks a lot! done | |
| 16:23:00 | fried_rice | belmoreira: Based on: 1700 iterations of the loop; each loop is doing three set_*_for_provider()s; and each set_*_for_provider is asking ProviderTree.has_*_changed() | |
| 16:23:32 | fried_rice | (ugh, my IRC client made a mess of that) | |
| 16:24:07 | fried_rice | ohhh | |
| 16:24:13 | fried_rice | So, here's a thing. | |
| 16:24:19 | fried_rice | The roots are stored in a list | |
| 16:24:32 | fried_rice | which we're iterating over to find the one we're asking about | |
| 16:24:50 | fried_rice | O(N) just to find the provider to do the compare on. | |
| 16:24:57 | fried_rice | So O(N^2) for that loop. | |
| 16:25:00 | fried_rice | Times three. | |
| 16:25:19 | fried_rice | belmoreira: You have a way to shove in a patch and test this easily? | |
| 16:25:33 | tssurya | dansmith, mriedem: yes !!! thanks a lot for the reviews on the API changes :) and sorry for giving such a hard time on that series :D but I am seriously happy that's done. I will respin something for the nits | |
| 16:25:39 | gibi | leakypipes, fried_rice: thanks | |
| 16:25:53 | belmoreira | fried_rice: I just did it... in the | |
| 16:25:58 | mriedem | tssurya: no problem, it's a complicated change | |
| 16:26:21 | mriedem | now just figure out how to not grind the API on startup when there is a down cell | |
| 16:26:22 | mriedem | :) | |
| 16:26:31 | tssurya | haha yea | |
| 16:27:58 | dansmith | tssurya: np, thanks for your persistence | |
| 16:28:02 | belmoreira | efried_rice: for the "updated_from_provider_tree" I'm also passing the compute_node and removed the last "for". It only to update/check that resource provider | |
| 16:29:15 | fried_rice | belmoreira: That would be one way to do it. | |
| 16:29:24 | fried_rice | belmoreira: care to push that patch? | |
| 16:30:15 | fried_rice | belmoreira: But I don't think that's a generic solution long-term. I think it's going to break as soon as we have shared storage providers, for example. | |
| 16:30:30 | fried_rice | belmoreira: Let me play with the efficiency of ProviderTree itself and see if I can't get that loop a lot tighter... | |
| 16:31:04 | belmoreira | fried_rice: ohh no! this was emergency mode :) just to keep Ironic alive | |
| 16:31:23 | fried_rice | Code blue, get a crash cart in here, got it. | |
| 16:36:12 | melwitt | le sigh, the ceph job is consistently timing out taking too long to run. not sure what could have changed | |
| 16:40:58 | fried_rice | belmoreira: are you running py2 or py3? | |
| 16:41:21 | belmoreira | fried_rice: py2 | |
| 16:41:30 | fried_rice | ight | |
| 16:42:40 | belmoreira | v | |
| 16:42:44 | melwitt | dansmith: your review on the spec amendment for detach boot volume would be appreciated https://review.openstack.org/619161 | |
| 16:43:10 | dansmith | melwitt: appreciated how much? | |
| 16:43:59 | melwitt | I have chicago style popcorn | |
| 16:44:35 | dansmith | yeah, alright. | |
| 16:45:50 | melwitt | lol | |
| 16:48:55 | openstackgerrit | Merged openstack/nova master: Make VolumeAttachmentsSampleV249 test other methods https://review.openstack.org/633621 | |
| 16:49:04 | belmoreira | I need to leave, thanks for all the help | |
| 16:54:04 | openstackgerrit | Merged openstack/nova-specs master: Amend the detach-boot-volume design https://review.openstack.org/619161 | |
| 17:00:34 | mriedem | melwitt: for the same reasons the tempest-full job times out | |
| 17:00:49 | mriedem | http://status.openstack.org/elastic-recheck/#1783405 | |
| 17:00:55 | mriedem | https://review.openstack.org/#/q/topic:bug/1783405+(status:open+OR+status:merged) | |
| 17:01:10 | mriedem | {1} tempest.scenario.test_network_advanced_server_ops.TestNetworkAdvancedServerOps.test_server_connectivity_reboot [278.769116s] ... ok | |
| 17:01:18 | mriedem | {1} tempest.scenario.test_volume_boot_pattern.TestVolumeBootPattern.test_volume_boot_pattern [231.855718s] ... ok | |
| 17:01:27 | mriedem | {2} tempest.api.compute.volumes.test_attach_volume.AttachVolumeTestJSON.test_attach_detach_volume [224.248984s] ... ok | |
| 17:01:51 | mriedem | {0} tempest.scenario.test_minimum_basic.TestMinimumBasicScenario.test_minimum_basic_scenario [266.396758s] ... ok | |
| 17:02:49 | melwitt | oh, I see. thank you. I didn't know the test times are on the tests | |
| 17:02:51 | mriedem | actually it would be worse | |
| 17:02:58 | mriedem | because tempest-full skips slow tests | |
| 17:03:00 | mriedem | but the ceph job doesn't | |
| 17:03:15 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP Use migration_status during volume migrating and retyping https://review.openstack.org/637224 | |
| 17:03:18 | mriedem | and the ceph job is also running cinder tempest plugin tests | |
| 17:03:38 | melwitt | the ceph job does it all! | |
| 17:03:41 | mriedem | http://git.openstack.org/cgit/openstack/devstack-plugin-ceph/tree/.zuul.yaml#n16 | |
| 17:03:47 | mriedem | ^ should be increased probably | |
| 17:03:53 | melwitt | it was fine until recently, now it's timing out constantly | |
| 17:03:58 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Perf: Use dicts for ProviderTree roots https://review.openstack.org/637225 | |