Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-10
14:59:41 gibi mriedem: it seems that the top of the bw series hits the race so I'm wondering what to do. Rebase the series to inclued the fix in the bottom, or wait for the grenade test result first as gate is slow
14:59:57 dansmith efried: are you talking about vpmem or PCPU?
15:00:39 efried dansmith: vpmem
15:01:18 efried bbiab
15:03:02 mriedem gibi: a nit inline on that race test fix
15:03:36 mriedem gibi: i think we'll just get the race fix merged today and then it's the same as you rebasing your series on it
15:04:01 gibi mriedem: OK. will put back the lower() call.
15:05:18 gibi mriedem: ahh good point, the gate will rebase the series for the test anyhow
15:07:44 mriedem gibi: left another comment, don't know if you want to fup again or not,
15:08:06 mriedem but since you changed all of these copy/paste blocks of confirmResize/wait for status/wait for event, those could go into a single helper method
15:09:17 gibi mriedem: OK I will put that in a helper. Do you suggest to only change the new confirm resize tests?
15:09:38 mriedem i'm not sure what you mean by 'new confirm resize tests'
15:10:08 mriedem like the new bw provider migration tests?
15:10:17 gibi mriedem: the current patch adds the instance action event waiting for every confirm resize tests in test_servers
15:10:38 gibi mriedem: not just to the bw related confirm resize tests
15:10:55 mriedem yeah i didn't expect to see all of those other changes in tests that aren't doing exotic allocatoins
15:11:05 mriedem i mean, it's fine, and we could just put the helper in ServerMovingTests
15:11:47 gibi this race does not depend on any exotic allocation.
15:12:15 gibi true we only saw the issue in the bw related tests
15:12:51 mriedem putting it in ServerMovingTests is probably not good enough then, since the new tests don't use that
15:12:54 mriedem in PortResourceRequestBasedSchedulingTestBase
15:13:02 mriedem could go in ProviderUsageBaseTestCase though...
15:13:36 mriedem up to you - i just don't love the copy/paste so we should use a helper at some point, fup or whatever - up to you
15:13:48 gibi mriedem: OK. I will respin the patch
15:14:28 openstackgerrit Artom Lifshitz proposed openstack/nova master: NUMA live migration support https://review.opendev.org/634606
15:14:29 openstackgerrit Artom Lifshitz proposed openstack/nova master: Deprecate CONF.workarounds.enable_numa_live_migration https://review.opendev.org/640021
15:14:29 openstackgerrit Artom Lifshitz proposed openstack/nova master: Functional tests for NUMA live migration https://review.opendev.org/672595
15:14:50 bauzas gibi: could you please give me again the race fix change ?
15:14:58 bauzas gibi: can't easily find it
15:15:03 gibi bauzas: https://review.opendev.org/#/c/681238
15:15:08 bauzas gibi: thanks
15:15:34 bauzas ah, that's because the topic name changed :)
15:16:15 sean-k-mooney alex_xu: luyao have you tested the vpmem code with instace that request hugepage or pinning but dont specify a numa toplogy
15:17:23 artom dansmith, mriedem, ^^
15:18:00 bauzas gibi: so you'll respin https://review.opendev.org/#/c/681238 ?
15:18:06 alex_xu sean-k-mooney: I guess luyao have test on that. But that will be a case like normal instance with numa. Since when we specify pinning or hugepage, we will create one node numa for the instance, right?
15:19:01 sean-k-mooney alex_xu: we should but i need to make sure https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py@4664 does nto break that
15:19:44 sean-k-mooney e.g. and instance with hw:cpu_policy=dedicated + vpmem needs to be pinned
15:19:59 sean-k-mooney and i need to ensure adding not need_pin wont break that
15:20:19 alex_xu sean-k-mooney: https://review.opendev.org/#/c/678455/25/nova/virt/libvirt/driver.py@5454, for that case, the need_pin should be True, since the instance has numa_topology
15:20:31 sean-k-mooney instance.numa_topology shoudl be set in the calim but i need to triple check
15:20:41 alex_xu yea
15:20:53 gibi bauzas: yeah, I will respin soon.
15:21:09 bauzas cool
15:21:22 alex_xu sean-k-mooney: actually we set instance.numa_topology in the API layer I think
15:21:30 sean-k-mooney alex_xu: i think the code is correct but im not sure im happy with this be in a diffreent location then the other places we create a numa toplogy implcitly
15:22:15 alex_xu sean-k-mooney: yea, that is agreement on the PTG, people said they want the vpmem on no numa instance, but I totally understand your point
15:23:01 alex_xu I can't remember who is asking, but it should be redhat people
15:23:46 sean-k-mooney well as a redhat person i did not but i think maybe dansmith prefered supporting this. i kown we said we woudl do that at the PTG however
15:24:16 alex_xu yea, actually I'm on your side in the beginning
15:24:17 sean-k-mooney alex_xu: do you have an xml i can inspect
15:24:42 dansmith sean-k-mooney: um, what?
15:24:44 sean-k-mooney e.g. an exmaple xml that was generated using this code
15:25:11 alex_xu sean-k-mooney: luyao has, but she is on the way home~
15:25:14 sean-k-mooney dansmith: you prefered to add pmem suport without requireing numa in the inital version corect?
15:25:29 sean-k-mooney dansmith: i could be miss rememebring if so my applogies
15:26:13 sean-k-mooney dansmith: i just rememebr you and i were the most active redhatters in that discuession in the ptg.
15:26:43 alex_xu sean-k-mooney: here is one http://52.27.155.124/95/674895/35/check/pmem-tempest-plugin-filtered/af5fdef/controller/logs/screen-n-cpu.txt.gz
15:27:06 alex_xu sean-k-mooney: search the 'pmem', you will see the xml output by nova-compute log
15:27:34 dansmith sean-k-mooney: I seriously doubt I said that
15:29:22 sean-k-mooney dansmith: in that case my applogies for invoking your name :)
15:29:34 sean-k-mooney alex_xu: so this is the generated domain
15:29:36 sean-k-mooney http://paste.openstack.org/show/774840/
15:30:06 openstackgerrit Balazs Gibizer proposed openstack/nova master: Fix the race in confirm resize func test https://review.opendev.org/681238
15:30:13 gibi mriedem, bauzas: ^^
15:30:58 sean-k-mooney alex_xu: it is both pinning ram of the and the core of the guest to float over host numa node 0
15:31:23 alex_xu sean-k-mooney: yes, but I think the is based on old patchset, since that ci job always pull a old version of luyao's patch
15:31:34 alex_xu in new version, it shouldn't have the pinning
15:31:35 sean-k-mooney alex_xu: but it is not specificy a constrait on pmem device as far as i can see
15:33:05 alex_xu sean-k-mooney: yes, that is the output for patchset13, sorry
15:33:24 sean-k-mooney alex_xu: im not sure that is a good thing. but ok. do you have an updated run
15:33:29 sean-k-mooney ill check the ci logs i guess
15:33:48 alex_xu sean-k-mooney: luyao is trying to get one for you
15:34:04 sean-k-mooney http://52.27.155.124/93/671793/23/check/pmem-tempest-plugin-filtered/23b32fa/ shoudl have them right
15:34:28 sean-k-mooney that from ps 23
15:34:49 sean-k-mooney i found one
15:35:03 alex_xu sean-k-mooney: no, it won't, the CI always try to apply the PS13, Rui is trying to remove that. That is why you can see other patch also can pass the pmem ci test
15:37:04 sean-k-mooney in the new code will both the numatune and cputune element be removed if only a pmem device is requested in the flavor
15:39:19 kashyap sean-k-mooney: To my eyes this whole NUMA and PMEM interaction looks sutble enough that some more functional testing is required...
15:41:48 sean-k-mooney kashyap: well there are functional tests in https://review.opendev.org/#/c/678470/27
15:41:53 sean-k-mooney but i have not looked at them yet
15:42:50 kashyap sean-k-mooney: Ah, missed it; thx
15:43:59 sean-k-mooney ... its locking up my firefox windwo trying to open it for some reason
15:44:16 bauzas gibi: https://review.opendev.org/#/c/681238/3
15:44:22 bauzas gibi: can I update the commit msg ?
15:44:45 sean-k-mooney kashyap: the functional test will not assert anythin about the xml generation
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

Earlier   Later