Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-15
15:49:44 gibi jaypipes: if only one resource class would be used then yes, but if you have two resource classes requested then which one would sorted first?
15:50:23 fried_rice jaypipes: I didn't feel strongly enough to say so in the review, but fwiw I liked gibi's algorithm as is.
15:50:24 jaypipes gibi: do we have tests that show the behaviour or your algorithm is correct for such situations?
15:50:24 belmoreira fried_rice: thanks, I'm checking
15:50:35 tssurya fried_rice: porting was done after that patch got merged
15:50:41 jaypipes fried_rice: it's difficult to read and understand, IMHO.
15:50:44 fried_rice tssurya: roger that, thanks.
15:51:03 fried_rice jaypipes: I don't disagree, but I'm not sure yours is more readable/understandable. (It is to you because you formulated it.)
15:51:15 jaypipes fried_rice: as is most things around multiple use_single_provider request groups, FWIW
15:51:22 gibi jaypipes: it worth to step through https://review.openstack.org/#/c/616239/26/nova/tests/unit/objects/test_request_spec.py@1305
15:52:05 fried_rice tssurya: How far up the series did you port?
15:52:41 gibi jaypipes: there group1 could be fit first to 3 RPs but finally only the RP #3 leads to solution
15:54:18 jaypipes fried_rice, gibi: this all just leaves an odd taste in my mouth.
15:54:44 jaypipes fried_rice, gibi: a taste that says "oh, we're totally leaking the placement selection details out of the placement API"
15:54:51 gibi jaypipes: totally agree
15:54:53 jaypipes and yes, I understand this is a workaround
15:54:57 fried_rice jaypipes: Yeah, it sucks, but is hopefully temporary, to be whacked when placement lets ... just so
15:55:20 jaypipes until such point that the result of allocation_candidates contains an indicator of which request group "belongs" to which allocation provider
15:55:34 gibi jaypipes: yes
15:55:36 fried_rice So let's get that spec pushed through so we can implement it early in Train and get rid of this spaghetti.
15:55:44 jaypipes ack, ok
15:56:06 gibi fried_rice: unfortunately I cannot focus both on that spec and the bandwidth series at the same time (I'm limited)
15:56:14 gibi fried_rice: but after feature freeze I can update the spec
15:56:15 fried_rice gibi: Would you like me to take over that spec?
15:56:34 jaypipes gibi, fried_rice: question for you... are different request groups containing same resource class requests guaranteed to land on different resource providers?
15:56:40 gibi fried_rice: if it is OK to push it after the feature freeze then I'd like to keep the spec
15:56:46 jaypipes (I can never remember...)
15:56:49 fried_rice jaypipes: if group_policy=none, no
15:57:02 jaypipes ah, yes, group_policy=isolate..
15:57:28 jaypipes gibi, fried_rice: is group_policy=isolate taken account of in this mapping patch?
15:57:29 fried_rice gibi: okay. Yes, that should be fine. Just lmk if you want help (other than review)
15:57:40 fried_rice jaypipes: oo, good question.
15:57:44 jaypipes or does it even need to be? /shurg
15:57:46 gibi fried_rice: thanks, I will need you review! :)
15:58:06 fried_rice jaypipes: I think it might need to be, yes.
15:58:07 gibi jaypipes, fried_rice: bahh, good question
15:58:13 belmoreira fried_rice: what is taking time is the last for in the update_from_provider_tree
15:58:47 fried_rice as is, gibi's algorithm takes the first of many possible matchups; but it assumes group_policy=none
15:58:58 jaypipes yeah
15:59:01 belmoreira https://github.com/openstack/nova/blob/d231a420d0d4865bb19da513af416cb8bc89010f/nova/scheduler/client/report.py#L1439
15:59:03 gibi fried_rice, jaypipes: if the request was made with isolate but the algo founds an overlapping solution then we are screwed
15:59:17 gibi fried_rice: correct
15:59:28 gibi fried_rice: so I have to enhance this algo
15:59:32 gibi fried_rice: to handle isolate
15:59:48 gibi fried_rice: and in that case throw away overlapping solutions
16:00:04 fried_rice gibi: correct
16:00:08 jaypipes gibi, fried_rice: well, maybe... maybe not. :) after all, we *KNOW* that the returned allocations properly meet the group_policy=isolate constraint because the placement service is shown to be correct in that.
16:00:58 fried_rice mm, nooo, I still think there's a hole
16:01:05 jaypipes so we can assume that the provider that is assigned a particular amount of allocated resources does not have any "doubled up" allocations from multiple request groups, right?
16:01:47 gibi in case of isolate, yes
16:01:50 fried_rice belmoreira: noted
16:01:56 jaypipes this is what's "eww" about this... :) it's essentially needing to re-implement the entire request group selection algorithm that's in the placement service already. :)
16:02:06 fried_rice belmoreira: because new_uuids == {every ironic node}
16:02:12 fried_rice jaypipes: Exactly.
16:02:19 gibi jaypipes: you are totatlly right
16:02:21 belmoreira fried_rice: correct
16:02:23 fried_rice jaypipes: and we haven't even done forbidden traits and aggregates yet
16:02:40 gibi fried_rice: and I hope we never have to in this algo
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.

Earlier   Later