| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-31 | |||
| 16:09:18 | mriedem | i also noticed https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:hacking-update this morning and it'd be nice, as a courtesy to the core team and others working on nova, to at least raise some of this stuff up in the ML or meeting *before* starting work on it | |
| 16:09:56 | mriedem | i'm not saying i'm against those hacking changes fwiw, just saying they are big changes that once merged will impact everyone so a heads up would be nice | |
| 16:11:24 | mriedem | while i'm ranting, can i get some eyes on https://review.opendev.org/#/q/topic:bug/1830747+(status:open+OR+status:merged) since bauzas is on EU PTO infinitum so i can start backports since that's going to hammer anyone with rescheduled cold migrations / resizes before stein, which is pretty gd nasty | |
| 16:11:38 | mriedem | dansmith: ^ you'd probably be best besides bauzas | |
| 16:11:40 | mriedem | if you're around | |
| 16:12:00 | dansmith | ack | |
| 16:12:15 | mriedem | danke | |
| 16:12:53 | mriedem | and welcome back to the fun house | |
| 16:13:50 | dansmith | heh thanks | |
| 16:16:19 | stephenfin | mriedem: I get where you're coming from but I really don't agree. If I'm happy to burn an hour on tech debt reduction, my patches are reviewable, and I keep them rebased as conflicts pop up, why not? EAFP and all that | |
| 16:16:40 | stephenfin | hacking being a fair exception since that touches a _lot_ of stuff, sadly | |
| 16:17:00 | stephenfin | I left the comments on https://review.opendev.org/#/c/662501/1/nova/api/ec2/ec2utils.py anyway. That -2 can probably be removed unless I've missed something obvious | |
| 16:27:01 | dansmith | mriedem_hangry: question I'll probably regret asking in that set | |
| 16:36:15 | sean-k-mooney | stephenfin: are you looking at the resize issue | |
| 16:36:27 | stephenfin | sean-k-mooney: I am | |
| 16:36:35 | stephenfin | sean-k-mooney: Are you? | |
| 16:36:49 | sean-k-mooney | no i know why it happens | |
| 16:36:58 | stephenfin | shoot | |
| 16:37:03 | sean-k-mooney | i looked at the code a few days ago | |
| 16:37:24 | sean-k-mooney | and asked them to file a bz which you triaged but i have filed an upstream bug | |
| 16:37:34 | stephenfin | Got a link? | |
| 16:37:35 | sean-k-mooney | if you want to fix it feel free | |
| 16:37:48 | sean-k-mooney | *have not | |
| 16:38:50 | stephenfin | sean-k-mooney: Want to give the tl;dr? | |
| 16:39:20 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1038 | |
| 16:39:37 | sean-k-mooney | we use the pagesize form the embeded instance numa toplogy object | |
| 16:40:22 | sean-k-mooney | so in a resize when we do https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L98 | |
| 16:40:33 | stephenfin | sean-k-mooney: Right, which we have one of already https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L74 | |
| 16:40:35 | stephenfin | yeah | |
| 16:40:51 | stephenfin | Why doesn't this hit rebuild though? | |
| 16:41:13 | sean-k-mooney | because its a resize | |
| 16:41:16 | stephenfin | Oh, because the size hasn't changed | |
| 16:41:18 | stephenfin | Yah | |
| 16:41:20 | stephenfin | *yeah | |
| 16:41:29 | stephenfin | *mempage size | |
| 16:42:16 | sean-k-mooney | anyway for a resize we need to update the instance toplogy object with the mem_page_size form the new flavor | |
| 16:42:43 | sean-k-mooney | so we can either make a copy of the toplogy and pass that in our maybe use the tempory_mutation decorator | |
| 16:42:56 | stephenfin | I'm trying to figure out what sets it for the requested_topology the first day (i.e. on a server create) | |
| 16:43:34 | sean-k-mooney | i belive we generate it form the flavor but dont fill in all the fields | |
| 16:43:45 | sean-k-mooney | but i didnt look | |
| 16:44:13 | sean-k-mooney | its foming from the sepc_object whic is the request_spec | |
| 16:44:48 | stephenfin | Must be https://github.com/openstack/nova/blob/master/nova/compute/api.py#L978 | |
| 16:45:50 | stephenfin | which is set by https://github.com/openstack/nova/blob/master/nova/compute/api.py#L876-L877 | |
| 16:50:37 | stephenfin | sean-k-mooney: https://bugs.launchpad.net/nova/+bug/1831269 | |
| 16:50:38 | openstack | Launchpad bug 1831269 in OpenStack Compute (nova) "Resize ignores mem_page_size in new flavor" [Undecided,New] | |
| 16:50:48 | sean-k-mooney | we should proably be updating the request spec here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3558-L3560 | |
| 16:51:48 | sean-k-mooney | which is just before we call resize instnace | |
| 16:52:08 | sean-k-mooney | that or self.compute_task_api.resize_instance should do it | |
| 17:01:41 | sean-k-mooney | stephenfin: i have triage it upstream and commented on the issue https://bugs.launchpad.net/nova/+bug/1831269 | |
| 17:01:42 | openstack | Launchpad bug 1831269 in OpenStack Compute (nova) "Resize ignores mem_page_size in new flavor" [Medium,Confirmed] | |
| 17:02:17 | sean-k-mooney | stephenfin: it should in theory be a fairly simple fix | |
| 17:02:45 | stephenfin | Yup, it should | |
| 17:03:08 | sean-k-mooney | i think it should be backportable too | |
| 17:03:18 | sean-k-mooney | rebuild is harder to fix | |
| 17:03:45 | sean-k-mooney | or rather inplace rebuild without the double allocations | |
| 17:03:45 | stephenfin | I'm not sure why this is passing though https://review.rdoproject.org/r/#/c/11297/11/whitebox_tempest_plugin/api/compute/test_cpu_pinning.py@108 | |
| 17:04:24 | sean-k-mooney | the downstream bug only happens because the destination host did not have any 2mb hugepages only 1G | |
| 17:04:27 | stephenfin | Actually, maybe it is. But we have a host that actually has enough free CPUs | |
| 17:04:40 | kashyap | efried: aspiers: Thanks for the reviews. | |
| 17:04:52 | stephenfin | So we'd start seeing this more when cpu-resources is done, because we might only have limited PCPU inventory | |
| 17:04:55 | aspiers | kashyap: we still need one more +2 | |
| 17:05:09 | aspiers | I guess we should wait for cdent | |
| 17:05:10 | kashyap | aspiers: Right, not sure who can give it. Chris Dent? | |
| 17:05:15 | kashyap | Bingo | |
| 17:05:27 | stephenfin | oooh, review.rdoproject.org supports polygerrit now | |
| 17:05:28 | sean-k-mooney | stephenfin we likely are looking up the cpu policy differently | |
| 17:05:30 | stephenfin | pretty | |
| 17:05:52 | aspiers | stephenfin: still waiting for OpenStack Gerrit upgrade | |
| 17:06:27 | stephenfin | yup, but I'm assured they're on it. Looking forward to a few of those features | |
| 17:06:32 | sean-k-mooney | nope https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1100 | |
| 17:06:38 | aspiers | stephenfin: but even that upgrade is fairly old: 2.14.7, and 3.0 is now out | |
| 17:06:55 | aspiers | there are lots of really nice improvements since 2.14 | |
| 17:07:13 | stephenfin | Yeah, I think we discussed before and the strategy was to go to 2.16, which deprecates a load of things but would provide time to prepare for 3.0 | |
| 17:07:28 | aspiers | stephenfin: skip upgrades are not supported anyway | |
| 17:07:42 | aspiers | you have to go to 2.16 to migrate to notedb before moving to 3.0 | |
| 17:07:48 | stephenfin | yup | |
| 17:08:07 | efried | stephenfin: Not sure how much mikal is monitoring his reviews these days; if I do those two fixups (remove redundant lines) would you fast approve? | |
| 17:08:34 | sean-k-mooney | stephenfin: so ya https://review.rdoproject.org/r/#/c/11297/11/whitebox_tempest_plugin/api/compute/test_cpu_pinning.py@108 should fail | |
| 17:08:47 | stephenfin | efried: I would | |
| 17:09:01 | stephenfin | sean-k-mooney: I'm thinking it would fail if the other had no free cores | |
| 17:09:04 | stephenfin | *other host | |
| 17:09:33 | sean-k-mooney | stephenfin: the self.get_server_cpu_pinning call should be checking the xml | |
| 17:09:38 | stephenfin | but because it does, the NUMATopologyFilter is passing and then things get recalculated properly when it starts building the instance on the host | |
| 17:09:56 | sean-k-mooney | unless we actully update the request spec later | |
| 17:10:06 | sean-k-mooney | e.g. on the compute node | |
| 17:10:19 | sean-k-mooney | right | |
| 17:10:36 | sean-k-mooney | ok so ya that is passing because there is enough free cpus | |
| 17:11:05 | sean-k-mooney | ok so resize with numa is not totally broken | |
| 17:11:15 | sean-k-mooney | just the sechduler bit | |
| 17:11:37 | sean-k-mooney | the compute node will still validate it with the new flavor when it trys to build the instance xml | |
| 17:12:28 | sean-k-mooney | or just before it when it generates the new numa toplogy | |
| 17:14:33 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: Recalculate 'RequestSpec.numa_topology' on resize https://review.opendev.org/662522 | |
| 17:14:56 | stephenfin | sean-k-mooney: ^ untested (completely) but something like that should do the trick, I imagine? | |
| 17:15:22 | sean-k-mooney | yes that is exactly what i was thinking too | |
| 17:15:44 | stephenfin | Nice | |
| 17:15:57 | sean-k-mooney | the one thing im not sure about however is should we but using the tempory mutation thing | |
| 17:16:12 | stephenfin | I'll try whip up a functional test for the on Monday^H^H^H Tuesday and see if I can reproduce it there | |
| 17:16:15 | sean-k-mooney | e.g. if we revert we need to make sure we go back to the old numa toplogy | |
| 17:16:15 | stephenfin | Possibly | |
| 17:16:45 | stephenfin | The fact that we're overwriting some RequestSpec fields already leads me to think we might be rolling back somewhere, but I haven't investigated | |