| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-10 | |||
| 20:01:25 | melwitt | mriedem pointed that out earlier | |
| 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: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 | melwitt | I dunno where that saying comes from actually | |
| 20:03:51 | mriedem | sean-k-mooney: whatever, that's better than nothign | |
| 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 add/remove instance info from node https://review.opendev.org/659691 | |
| 21:50:02 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK for getting network metadata from node https://review.opendev.org/670213 | |
| 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 | |
| 00:40:42 | melwitt | sean-k-mooney: lol, looks like my beloved auto topic from git-review removed my good branch name | |
| 00:40:47 | sean-k-mooney | for a stine node we would usee the old logic | |
| 00:41:02 | sean-k-mooney | melwitt: i think that is why it was removed | |