| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-10 | |||
| 20:00:40 | melwitt | big delta from legacy counting and only multi-cell ppl likely to "need" it (for the down cell resilience). so we chose not to impose the big delta in behavior on the majority who don't need down cell resilience | |
| 20:01:08 | sean-k-mooney | so do we just want to add another line here https://github.com/openstack/nova/blob/f4ca3e70852c0a7ed7904a9f2d7177c9118d3d1c/nova/scheduler/client/report.py#L2356 to make it work by combining them? | |
| 20:01:22 | sean-k-mooney | *them being PCPUs | |
| 20:01:25 | mriedem | to be fair though, some of those differences in behavior in counting also changed since i think ocata or pike | |
| 20:01:25 | melwitt | mriedem pointed that out earlier | |
| 20:01:41 | mriedem | when we moved to counting in general and dropped reservations | |
| 20:01:51 | mriedem | and no one apparently noticed until we noticed in stein :) | |
| 20:01:52 | sean-k-mooney | yes im wondering if we should comment tha ton stephens patch as the way forward so he can do it tomorrow | |
| 20:02:21 | melwitt | mriedem: I think there was only one change in pike, doesn't leave room for a revert resize, IIRC (I documented all of it on the review comment) | |
| 20:02:24 | mriedem | sean-k-mooney: i think one could say, "one way forward is ...." | |
| 20:02:38 | melwitt | the new stuff, there's way more deltas than that, a big laundry list | |
| 20:03:23 | mriedem | what is a laundry list anyway? 1. put stuff in washer and wash, 2. put stuff in dryer, 3. fold. wouldn't a more accurate list be a grocery list? | |
| 20:03:29 | sean-k-mooney | mriedem: ok but stephen isnt here and i wanted to provide a summary to him but ill just tell him to read scollback in the morning | |
| 20:03:37 | melwitt | mriedem: hah | |
| 20:03:51 | mriedem | sean-k-mooney: whatever, that's better than nothign | |
| 20:03:51 | melwitt | I dunno where that saying comes from actually | |
| 20:04:08 | mriedem | https://english.stackexchange.com/questions/437507/what-is-the-origin-of-the-phrase-laundry-list | |
| 20:04:22 | melwitt | I still haven't wrapped my head around what "combining" the resource classes means and whether/how it's different from legacy counting | |
| 20:04:33 | mriedem | it wouldn't be different from legacy counting | |
| 20:04:35 | melwitt | I mean, I know it means VCPU + PCPU | |
| 20:04:40 | melwitt | ok | |
| 20:04:42 | mriedem | so i think the summary is, | |
| 20:05:05 | mriedem | 1. with legacy counting, pcpu is counted the same since we use instance.vcpus which comes from the flavor | |
| 20:05:16 | mriedem | 2. with placement counting, we'd have to combine VCPU and PCPU usage to match ^ | |
| 20:05:26 | dansmith | 3. neither are actually right | |
| 20:05:35 | mriedem | 3. long-term combining them goes against the goal of separate them as countable trackable resources | |
| 20:05:39 | mriedem | right | |
| 20:06:28 | mriedem | so the immediate question is, is #2 good enough for train with punting sorting out #3 to the future | |
| 20:07:01 | sean-k-mooney | so this is a bit of nova i dont really fully understand. melwitt is there anythin we can do before unified limts to supprot not combining them if that is prefrable | |
| 20:07:16 | sean-k-mooney | or do we just wati for unifed limits to do #3 | |
| 20:07:59 | melwitt | I guess if it's not worse than what's possible with legacy counting (inability to separate) then the hack doesn't sound like a huge issue to me. it would obviously need a TODO on it to get rid of it when unified limit support exists and is enabled | |
| 20:09:59 | melwitt | sean-k-mooney: nothing reasonable, really. you'd have to add a new quota config and resource for "pcpu_whatever" and use that to let people set the limit separately. because until unified limits, there would be no way for nova to consume the unified limit for PCPU that a person sets in keystone | |
| 20:10:36 | sean-k-mooney | right and we dont really want to do that because techdebth | |
| 20:10:52 | efried | From where I sit (outside the swirling maelstrom of actual understanding) it sounds like we've had a pretty big imbalance "forever" where we've been counting dedicated and virtual cpus "the same" from a quota perspective. If so, we're not making it worse by continuing to do that, just now we're doing it with a separate resource class for the former. | |
| 20:10:53 | melwitt | yeah, that would be a big tech debt | |
| 20:11:21 | melwitt | whereas the hack of combining VCPU and PCPU doesn't sound like big debt to me, unless I've missed something more complex about it | |
| 20:11:30 | melwitt | "small debt" | |
| 20:13:07 | sean-k-mooney | it should be small to do i think. i assume its just the line mriedem linked but this code is new to me | |
| 20:13:19 | mriedem | plus functional testing | |
| 20:13:39 | sean-k-mooney | we need to haneld this branch too https://github.com/openstack/nova/blob/f4ca3e70852c0a7ed7904a9f2d7177c9118d3d1c/nova/scheduler/client/report.py#L2344 | |
| 20:13:52 | mriedem | i'd want to see a functional test that creates a server that allocates PCPU inventory and then assert that cores usage for that tenant is incremented | |
| 20:14:29 | sean-k-mooney | i think stephen has a function test for the first half so the quota check shoudl not be too hard to add | |
| 20:17:21 | sean-k-mooney | ok so he has functional test in the reshap patch at a minium | |
| 20:18:20 | sean-k-mooney | ok yes he add more here https://review.opendev.org/#/c/671801/43/nova/tests/functional/libvirt/test_numa_servers.py | |
| 20:18:48 | sean-k-mooney | so that proably where it makes sense to ad the test | |
| 20:41:19 | mriedem | artom: https://review.opendev.org/#/c/640021/48 | |
| 20:45:02 | mriedem | i probably won't get to the functional test patch tonight | |
| 20:46:45 | sean-k-mooney | mriedem: for what its worth i did test teh [upgrade_levels]/compute=stein case prviously | |
| 20:47:02 | sean-k-mooney | i can test it again tomorrow im more or less done for the day | |
| 20:47:22 | sean-k-mooney | btu if either node has [upgrade_levels]/compute=stein we endup with the stien behavior | |
| 20:47:40 | sean-k-mooney | the migration successes as long as the cores used in the souce host exist on the dest | |
| 20:47:45 | sean-k-mooney | but no xml is updated | |
| 20:47:57 | mriedem | so it goes back to the bug behavior | |
| 20:47:58 | mriedem | right? | |
| 20:48:03 | sean-k-mooney | yep | |
| 20:48:15 | sean-k-mooney | it goes back to the current master/stien behavior | |
| 20:48:20 | mriedem | yeah i don't know that we really need to bend over backward to try and detect that from conductor | |
| 20:49:05 | mriedem | if you're computes are fully upgraded and restarted and reporting as train service versions you should have also removed any manual pins | |
| 20:49:11 | sean-k-mooney | if you dont have can_live_migrate_numa set we will block it anyway | |
| 20:49:15 | mriedem | *your | |
| 20:49:44 | mriedem | not with the new checks | |
| 20:49:49 | mriedem | we would totally ignore can_live_migrate_numa | |
| 20:49:59 | mriedem | as long as all computes are reporting train | |
| 20:50:04 | sean-k-mooney | i thik we have a min compute servcie check too before we ignore it | |
| 20:50:20 | sean-k-mooney | oh thats the compute service | |
| 20:50:22 | sean-k-mooney | not the RPC | |
| 20:50:25 | mriedem | if we pass that check we ignore the config | |
| 20:50:30 | mriedem | right | |
| 20:51:09 | sean-k-mooney | well ya i agree we likely dont need to bend over backwards to check this in the conductor | |
| 20:52:10 | sean-k-mooney | anyway my concentration is totally gone so ill call it a day. | |
| 20:52:42 | sean-k-mooney | artom: ill test v 49 tommorow when you have addressed mriedem comments | |
| 21:10:33 | mriedem | melwitt: brinzhang has been asking me to review this api change https://review.opendev.org/#/c/674243/ but i haven't had time with the bw provider migrate and numa lm stuff - maybe you can peruse it? | |
| 21:11:09 | artom | mriedem, ack, thanks! | |
| 21:11:22 | artom | (The func test can wait until after FF if that's what it comes to, right?) | |
| 21:11:39 | artom | I mean, it's not plan A, but... | |
| 21:13:21 | mriedem | my priority is sean's manual testing and the gate job | |
| 21:13:50 | mriedem | brinzhang: a follow up is not the correct answer to all issues https://review.opendev.org/#/c/679413/4 | |
| 21:19:03 | melwitt | mriedem: will do | |
| 21:25:47 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for add/remove instance info from node https://review.opendev.org/659691 | |
| 21:25:48 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for getting network metadata from node https://review.opendev.org/670213 | |
| 21:50:02 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for getting network metadata from node https://review.opendev.org/670213 | |
| 21:50:02 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for add/remove instance info from node https://review.opendev.org/659691 | |
| 23:34:51 | openstackgerrit | Merged openstack/nova master: api-ref: fix server topology "host_numa_node" field param name https://review.opendev.org/680775 | |
| #openstack-nova - 2019-09-11 | |||
| 00:20:35 | melwitt | sean-k-mooney: fyi I've proposed a stable/stein release of os-vif. I didn't find one already proposed https://review.opendev.org/681358 | |
| 00:31:48 | sean-k-mooney | release or branch. there is a a stable branch but i dont think we have done a stabel release in a while | |
| 00:33:14 | sean-k-mooney | melwitt: in either case a release proably should be done as i dont think we have release since we fixed the cve with the linux bridge flooding | |
| 00:33:15 | alex_xu | sean-k-mooney: so we are still good at current upgrade plan for pcpu? I saw there a looong discussion on that | |
| 00:33:52 | sean-k-mooney | alex_xu: am i think dan wants to make the config outhion not required | |
| 00:34:17 | alex_xu | sean-k-mooney: ok, i see now. | |
| 00:34:26 | sean-k-mooney | by trying to claim with PCPUs and if that fails fall back to claiming with VCPUs and rely on the numa toplogy filter to validate it | |
| 00:34:34 | melwitt | sean-k-mooney: release, for stable branch. and correct, we've not had a release of the stable/stein branch since the cve fix | |
| 00:35:02 | sean-k-mooney | melwitt: ya so i proably should have proposed one when that merged. thanks for doing it | |
| 00:35:23 | alex_xu | sean-k-mooney: thanks, that is interesting... | |
| 00:35:45 | alex_xu | how can I know whether I need to stop fallback after upgrade | |
| 00:35:50 | melwitt | sean-k-mooney: np. I just wanted to make sure I didn't duplicate a release proposal in case I hadn't found it in my gerrit search | |
| 00:37:15 | sean-k-mooney | i think the release are ment to be on a topic branch with the project name but i dont see one looking quickly either | |
| 00:38:25 | sean-k-mooney | ya looking at all the open patches agains the release repo yours is the only os-vif one i see | |
| 00:39:16 | sean-k-mooney | alex_xu: basicailly how it would work is it would ould fallback if the first placmeent query with PCPUs failed then it would try with VCPUs | |
| 00:40:06 | sean-k-mooney | the numa toplogy filter could tell if the compute node was an train node or a pre train node based on the field in the host numa toplogy object | |
| 00:40:36 | sean-k-mooney | so on train nodes all either there ill be no pinned cpus or the will all be used | |