| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-21 | |||
| 19:56:10 | artom | Which the conductor *catches* | |
| 19:56:15 | artom | Or should, according to the code | |
| 19:56:23 | mriedem | is your test using the CastAsCall fixture? | |
| 19:57:52 | artom | Huh, do func tests use it by default? | |
| 19:58:09 | mriedem | no, but you might be extending a class that uses it | |
| 19:58:40 | artom | Ah, yeah, _IntegratedTestBase | |
| 19:58:50 | artom | Oh, that would be why the NoValidHost ends up in the API | |
| 19:59:17 | artom | Heh, can I "unuse" a single fixture? | |
| 20:14:56 | artom | No, there's something else. | |
| 20:15:18 | artom | It dies on the select_destinations call (which is always a call) in _find_destinations in the live migrate task | |
| 20:16:28 | artom | I... I think this exists in master as well. | |
| 20:30:32 | mriedem | jaypipes: so i'm not sure if my comments in here warrant a -1 or if it's just follow up fodder https://review.openstack.org/#/c/619528/ | |
| 20:30:56 | mriedem | but there are some serious performance impacts there if you're doing multi-create and not actually doing fancy port bw scheduling | |
| 20:32:44 | artom | Hrmm, or not. | |
| 20:32:59 | jaypipes | mriedem: multi-create meaning multiple instances in the same request, yeah? | |
| 20:33:04 | mriedem | yeah | |
| 20:33:09 | jaypipes | mriedem: mayhaps. | |
| 20:33:11 | mriedem | but this is even heavy for a single instance | |
| 20:33:25 | mriedem | post-scheduler, call placement to get allocations and traits for each provider in those allocations | |
| 20:33:32 | mriedem | for an instance that might not even have one of these fancy ports | |
| 20:33:34 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: De-shim PlacementFixture https://review.openstack.org/637959 | |
| 20:39:32 | jaypipes | mriedem: which is kinda what I said in the original review.. | |
| 20:39:57 | jaypipes | mriedem: but the alternative is to unpack the JSON in the Selection.allocation_request field blobified goop. | |
| 20:40:31 | mriedem | that won't work, | |
| 20:40:39 | mriedem | oh wait | |
| 20:40:47 | mriedem | maybe it will because that has the resource provider uuid in it right? | |
| 20:41:03 | mriedem | it's just a json version of https://developer.openstack.org/api-ref/placement/#update-allocations | |
| 20:41:22 | mriedem | yeah dude that's way faster than hitting placement for each instance | |
| 20:41:38 | mriedem | well, *should be* faster | |
| 20:44:53 | mriedem | that's also potentially the same Selection object (allocation_request) for multiple servers in the same build operation | |
| 20:45:19 | mriedem | so deserialize once rather than GET /allocations/{consumer_id} x number of instances | |
| 20:47:38 | jaypipes | mriedem: ack. | |
| 20:48:16 | mriedem | we can follow up also and avoid that GET /resource_provider/{rp_id}/traits call yeah? by stashing the GET /a_c provider_summaries traits per provider on the Selection object we return from the scheduler | |
| 20:48:17 | jaypipes | mriedem: which violates our original intention that the allocation_request would be a BLOBby thing that could be sent as-is as the request payload to placement on the event of a reschedule/retry operation] | |
| 20:48:20 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: De-shim PlacementFixture https://review.openstack.org/637959 | |
| 20:48:39 | jaypipes | mriedem: but as you say, it's considerably faster than re-querying placement for each instance | |
| 20:48:47 | mriedem | i'm not sure how it violates that | |
| 20:49:16 | mriedem | for the first selection in the list, it's allocatoin request has already been 'consumed' | |
| 20:49:18 | jaypipes | mriedem: well, it violates the principle that the allocation_request's structure could change and nothing in nova would need to be changed. | |
| 20:49:26 | jaypipes | mriedem: yes, understood. | |
| 20:49:43 | jaypipes | again, this was the original thought... things have changed/softened in our thinking since then | |
| 20:49:44 | jaypipes | :) | |
| 20:49:53 | mriedem | i'm not sure what you're saying :) | |
| 20:50:11 | mriedem | it's still a string on the selection object, in this code we just deserialize that field to get the rp uuids | |
| 20:50:22 | mriedem | well, the actual allocations i guess | |
| 20:50:28 | jaypipes | it's not just the rp uuids. | |
| 20:50:33 | jaypipes | yeah, it's all the allocation info | |
| 20:50:47 | mriedem | i guess what you're saying is we'd be treating the allocatoin request like a response | |
| 20:50:49 | mriedem | when it's not | |
| 20:51:04 | jaypipes | and of course, if we change the structure of the allocation_request response, boom, that "deserialize this blob and inspect it" code would of course need to change | |
| 20:51:11 | jaypipes | mriedem: yes. | |
| 20:51:37 | jaypipes | mriedem: I'm just trying to give you some background on why that's a JSON string and not an object :) | |
| 20:51:54 | mriedem | yeah i remember | |
| 20:52:00 | mriedem | and why we also provider the version | |
| 20:52:01 | jaypipes | ok, I will go away now | |
| 20:52:03 | mriedem | *provide | |
| 20:52:07 | mriedem | don't go away jay | |
| 20:52:09 | jaypipes | :P | |
| 20:52:09 | mriedem | please stay | |
| 20:52:29 | openstackgerrit | Merged openstack/nova stable/queens: Fix a missing policy in test policy data https://review.openstack.org/637112 | |
| 20:52:37 | openstackgerrit | Merged openstack/nova stable/rocky: tox: Don't write byte code (maybe) https://review.openstack.org/636917 | |
| 20:55:10 | mriedem | so maybe what i'll do is pull this down, put in the very easy "if the request spec doesn't have requested_resources, don't do anything with this new code" thing, leave a bunch of todos for gibi to follow up | |
| 20:55:24 | mriedem | that solves 99% of my performance worry | |
| 20:55:44 | mriedem | jaypipes: ^ how do you like that? | |
| 20:57:01 | efried | mriedem: That sounds like a pretty reasonable solution to me. And you would be doing it rather than letting gibi do it because... you want to get it merged today? | |
| 20:57:14 | mriedem | yes | |
| 20:57:21 | mriedem | time is as they say of the essence | |
| 20:57:44 | jaypipes | mriedem: ✔ | |
| 20:58:34 | efried | makes me a little nervous for the author of the patch not to have a nod on a change of that magnitude, logic-wise. | |
| 20:58:39 | efried | I guess worst case it can be reverted | |
| 20:58:47 | efried | but then again why couldn't it be done in a fup anyway? | |
| 21:00:03 | mriedem | i can be, but i just really don't want to get close to freeze with this like it is | |
| 21:00:21 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Add migration param to check_can_live_migrate_destination https://review.openstack.org/634605 | |
| 21:00:21 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Remove _legacy_dict methods https://review.openstack.org/636210 | |
| 21:00:22 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Use live_migration_claim() to check dest resources https://review.openstack.org/634606 | |
| 21:00:22 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Introduce live_migration_claim() https://review.openstack.org/635669 | |
| 21:00:23 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: LM: Make dest send NUMAMigrateData to the source https://review.openstack.org/634828 | |
| 21:00:23 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: New objects to transmit NUMA config from dest to source https://review.openstack.org/634827 | |
| 21:00:24 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: LM: update NUMA-related XML on the source https://review.openstack.org/635229 | |
| 21:00:53 | mriedem | btw the change i'm talking about is 1 line | |
| 21:01:13 | mriedem | do this https://review.openstack.org/#/c/616239/33/nova/objects/request_spec.py@781 | |
| 21:01:30 | mriedem | at the beginning of https://review.openstack.org/#/c/619528/27/nova/conductor/manager.py@1248 | |
| 21:02:03 | artom | efried, mriedem, hey, so I know it's late in the cycle, and I'd be trampling all over the runways process, but what are the chances of pushing for reviews/merge of ^^ before FF? | |
| 21:02:23 | jaypipes | mriedem: that is 2 lines Matt. Un. Friggin. Acceptable. | |
| 21:02:59 | mriedem | artom: there are more cores here than me | |
| 21:03:19 | mriedem | artom: at this point i'm pushing to get the bw provider stuff in before FF | |
| 21:03:25 | artom | mriedem, I know. You reviewed the spec is why. But I get that you're oversubscribed as is. | |
| 21:03:44 | sean-k-mooney | jaypipes: he did say about 1 line :P | |
| 21:04:07 | artom | Which is why I mentioned efried :) Intel are apparently keep on it. | |
| 21:04:51 | mriedem | artom: what's the test story on that series? is https://review.openstack.org/#/c/634606/24/nova/tests/functional/compute/test_live_migration.py the most functional we have? | |
| 21:05:02 | artom | mriedem, in-tree, yeah | |
| 21:05:20 | artom | I wrote https://review.rdoproject.org/r/#/c/18832/ that I've been using in my devstack | |
| 21:05:24 | efried | artom: I have been requested both internally and externally to have a look at that. Unfortunately, it's behind a number of other things, as well as requiring quite a bit of homework on my part just to grok the background. I think alex_xu is involved though, right? | |
| 21:05:51 | artom | efried, not really, you're our only hope (Obi Wan) | |
| 21:05:51 | jaypipes | sean-k-mooney: :P | |
| 21:06:51 | artom | So, it's easy for me to get stephenfin to look at it, being the same company and all | |
| 21:06:57 | sean-k-mooney | artom: the code freeze is still a few weeks away. we shoudl be able to get to it once bw feature is in | |
| 21:07:02 | artom | It's the other non-RH core that's the problem | |
| 21:07:16 | mriedem | i also have like a 50 change series of my own | |
| 21:07:34 | artom | ... and who groks NUMA? jaypipes I guess? But you're as oversubscribed as mriedem. | |