| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-15 | |||
| 15:45:51 | belmoreira | we couldn't cherrypick the patches in a clean way, we may have missed something | |
| 15:45:53 | fried_rice | belmoreira: Latest master has changes that made sure to only update the ironic node being touched. | |
| 15:46:05 | fried_rice | let me find the patch that did that... | |
| 15:47:20 | fried_rice | belmoreira: https://review.openstack.org/#/c/615677/ | |
| 15:49:01 | fried_rice | belmoreira: I think if you picked that up prior to about PS17, you would have the ironic explosion behavior you're seeing. But after that it *should* be fixed. | |
| 15:49:06 | jaypipes | gibi: hmm, couldn't my algorithm work if we just sorted the request groups in descending order of amount of resources being requested in the group? | |
| 15:49:24 | jaypipes | gibi: then the "greedy" approach would work fine, no? | |
| 15:49:40 | fried_rice | belmoreira: and I know you guys were picking up that series at various points along the way | |
| 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 | |