Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-10
15:44:46 gibi bauzas: sure go ahead
15:45:55 kashyap sean-k-mooney: Sure, I see those are already in the main change.
15:46:41 sean-k-mooney there are no assertion made about numa element in the main change as far as i can tell
15:46:54 sean-k-mooney rather numatune and cputune
15:48:07 bauzas gibi: cool, will do and +2
15:48:12 gibi bauzas: thanks a lot
15:48:41 mriedem gibi: nits in https://review.opendev.org/#/c/676140/19 for a follow up
15:48:52 openstackgerrit Sylvain Bauza proposed openstack/nova master: Fix the race in confirm resize func test https://review.opendev.org/681238
15:49:25 sean-k-mooney efried_afk: alex_xu ill try to come back to the pmem stuff in an hour or so. i need to review stephens patch then sync with artom
15:49:52 gibi mriedem: ack. thanks
15:50:07 sean-k-mooney stephenfin: you wanted me to review https://review.opendev.org/#/c/671793/ specifcally right. should i also review the rest
15:50:27 stephenfin sean-k-mooney: would be helpful, aye
15:50:35 alex_xu sean-k-mooney: i got one http://paste.openstack.org/show/774842/
15:50:37 stephenfin I'm marching through the pmem stuff
15:51:05 bauzas but I'll look at your series tomorrow morning
15:51:13 sean-k-mooney alex_xu: looking
15:51:16 artom sean-k-mooney, for once, I think I'm good, and do not require your excellent services :)
15:52:30 sean-k-mooney alex_xu: i think that that is correct. it is creatin a virtual numa toplogy but it is not tiying it to the host in any way
15:53:06 sean-k-mooney artom: the pmem device is also assocaitated with the virtual guest numa node 0
15:53:27 sean-k-mooney artom: are you working on fixing the persitence issue
15:54:05 sean-k-mooney artom: i jsut wanted to circle back and see if you had anything form me to test or if i should start looking into where to fix the issue
15:54:22 artom sean-k-mooney, I added instance.refresh() back it, so that's settled
15:54:31 alex_xu sean-k-mooney: but I think this is wrong https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py@5458
15:54:42 sean-k-mooney artom: ok did you push that?
15:54:51 artom The func tests weren't hitting it because driver.cleanup() is called conditionally, and the func test env isn't meeting those conditions
15:54:53 artom sean-k-mooney, I did push
15:55:03 alex_xu sean-k-mooney: I checked the nova show, I saw there is "hw:numa_nodes" being added, so I guess that is persistented in the db
15:55:06 sean-k-mooney ok then ill check it locally unless you have already
15:55:09 artom If I just change the code to always call it, I can reproduce
15:55:19 sean-k-mooney artom: that is not correct
15:55:20 artom And instance.refresh() does indeed fix it
15:55:27 artom sean-k-mooney, I know :) It was just to test
15:55:33 mriedem stephenfin: did you ever re-post your PCPU upgrade ML thread with [nova] tagged on it to actually get operator visibility?
15:55:42 artom Next step is to trigger driver.cleanup in the "real" way in func tests
15:55:48 sean-k-mooney we should not see hw:numa_nodes in nova show
15:55:59 alex_xu sean-k-mooney: yes, we shouldn't
15:56:01 sean-k-mooney artom: sorry that was for allex
15:56:12 sean-k-mooney artom: i have not looked at your change
15:56:19 stephenfin mriedem: No, it didn't seem necessary since we'd solved the upgrade issue in a way that didn't require anything special from the operator
15:56:28 stephenfin outside of bog standard config options
15:56:34 sean-k-mooney alex_xu: we should not see hw:numa_nodes=1 if its not in the flavor
15:56:51 sean-k-mooney alex_xu: we do not see that when you get implcit numa toplogyies in other cases
15:57:02 sean-k-mooney alex_xu: so if you are seeing it then the code is incorrect
15:57:08 artom sean-k-mooney, we're good, don't worry :)
15:57:17 mriedem stephenfin: isn't dansmith's comment all about a nasty upgrade problem?
15:57:26 alex_xu sean-k-mooney: I think the problem is the patch is change instance.flavor direclty, after driver.spawn, the nova-comptue update the instance object, then persistent it into the db.
15:57:28 mriedem to which operators, like mnaser, might want to weigh in?
15:57:32 sean-k-mooney alex_xu: if we were to cold migrate the instance the behavior sould chagne if we save it to the db
15:57:47 mnaser hm
15:57:53 dansmith mriedem: yeah, the "plan" doesn't seem super great to me as currently laid out :/
15:57:59 sean-k-mooney alex_xu: ya we shoudl not be changeing tehe flavor at all
15:58:13 stephenfin mriedem: an intractable one though. Even if operators don't like the little dance we're doing, I fail to see how there's an alternative
15:58:14 mriedem mnaser: you'd need someone to tl;dr it (i would also)
15:58:15 sean-k-mooney alex_xu: the other case dont chagne the flaovr they just create a numa toplogy
15:58:21 mriedem since there are 5 conversations going on at once in here right now
15:58:23 mnaser sounds like an ML post?
15:58:28 mnaser that i can read
15:58:37 mriedem mnaser: there was one which no operators read :)
15:58:43 mriedem b/c it was'nt tagged for [nova] or [ops]
15:58:43 sean-k-mooney so the pmem code is tacking a shortcut by updating the flavor. on a hard reboot that instance would be pinned
15:59:07 stephenfin sean-k-mooney: I'm looking at that at the moment. I don't like it.
15:59:09 stephenfin Not at all
15:59:31 stephenfin Assuming you're referring to https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py@5458
15:59:33 mriedem stephenfin: for the sake of everyone's clarity, could you post a new ML thread with the proposed upgrade path for PCPU and tag with [nova] and [ops]?
15:59:36 sean-k-mooney stephenfin: if we need to create a numa toplogy we should move it to where we do it for hugepages right?
15:59:56 sean-k-mooney stephenfin: yes
16:00:03 sean-k-mooney stephenfin: that is the hack that i dont like
16:00:05 stephenfin sean-k-mooney: exactly what I'm writing in a comment as we speak
16:00:19 stephenfin what is it with people trying to hack flavors :D
16:00:22 stephenfin mriedem: sure
16:00:35 stephenfin though I really don't see the point
16:00:42 dansmith stephenfin: "intractable" is a bit of a silly characterization :)
16:00:43 alex_xu sean-k-mooney: yea, agree with you
16:00:50 stephenfin because the only people that can solve this are in this channel/on the review already
16:01:42 sean-k-mooney stephenfin: the issue is that we want a numa topology in the xml. but not in the numa toplogy filter
16:01:55 stephenfin dansmith: Possibly :) I have been thinking about this for quite some time though and we've gone through a lot of options, so it starts looking like that to me, heh
16:03:16 sean-k-mooney alex_xu: since stephenfin is looking at it im gong to review his cpu code then ill come back to this after i test artoms code
16:03:19 mriedem stephenfin: saying "because the only people that can solve this are in this channel/on the review already" is not true imo - if you've got a hard upgrade thing coming for operators, you likely should get some feedback from them before pushing forward
16:03:28 alex_xu sean-k-mooney: thanks a lot
16:04:07 sean-k-mooney mriedem: the upgrade will be signifcantly harder if we also have to deal with numa in placment in the same release
16:04:26 mriedem sean-k-mooney: i don't know what that has to do with this at all
16:04:34 sean-k-mooney mriedem: if we defer pcpus in placment to U we will have to deal with both in one go
16:04:55 mriedem i didn't say anything about deferring
16:05:33 mriedem i said, does anyone outside of the 3 people reviewing this that will actually have to deal with the upgrade know what the plan is
16:05:36 mriedem and are they ok with it
16:05:39 sean-k-mooney right but the current upgrade approch is the best we could come up with and we went to the MLs and asked if the toggel was ok
16:05:54 mriedem and no operators even saw that thread,
16:06:06 mriedem which is why i asked (again) if it could be posed with a [nova][ops] tag
16:06:08 mriedem to get visibility
16:06:18 mriedem lack of feedback from operators is not agreement
16:06:50 sean-k-mooney well we did ask cern in irc
16:06:56 dansmith I think it's probably good to get feedback not just from ops,.
16:07:00 sean-k-mooney but i would have liked other to comment too
16:07:03 dansmith but from people that have to do this in the deployment tools
16:07:08 mriedem it would be a lot better to know before releasing train that "this sucks but it's not terrible" rather than "this is a no-go for me"
16:07:14 dansmith as this adds at least one more atomic reconfigure/restart of the deployment
16:07:49 mriedem sure, i lump mnaser into the ops and tooling (OSA) camps
16:08:00 dansmith yup
16:08:04 stephenfin I don't see what the actual issue is though
16:08:10 sean-k-mooney dansmith: for what its worth we talked about this internally with our tripleo folks that will be implementing and they were ok and actully prefered the seperate config flip step

Earlier   Later