Posted	Nick	Remark
#openstack-nova - 2019-09-11
15:00:53	dansmith	artom: ack
15:01:30	mriedem	artom: the code isn't granular to just the source and selected dest - where you're checking we don't have a dest yet in some cases,
15:01:37	mriedem	so i'm not sure why that would matter for the reno
15:01:41	mriedem	oh this is the log
15:01:58	artom	mriedem, yeah - I don't want to say "WARNING LEGACY BEHAVIOR" unconditionally
15:01:59	stephenfin	Yeah, the NUMA fitting code in the hardware module only gets InstanceNUMATopology, NUMATopology, PCIRequest and whatever the host PCI tracking object is
15:02:13	mriedem	artom: i think it's fine to say computes aren't new enough
15:02:17	mriedem	that's generic enough
15:02:32	artom	Yeah, except they *might* be new enough
15:02:37	mriedem	that doesn't matter
15:02:44	artom	If that specific source and dest are, even if others in the cell aren't
15:02:45	mriedem	b/c that's not how the code works
15:02:55	alex_xu	stephenfin: ah, you complain the part of explicltly numa but no affinity
15:02:56	mriedem	iow i don't see a problem with the paraphrase that dan gave
15:03:13	alex_xu	stephenfin: but you are ok with implicity numa?
15:03:21	artom	So, if we got passed the service check, it means there are some old computes in the cell
15:03:26	alex_xu	to be host, I'm confused what we discussion for now....
15:03:32	mriedem	artom: correct
15:03:35	artom	But... the actual source and dest might both be new
15:03:44	artom	In which case, assuming the workaround is enabled, NUMA-LM will happen
15:03:45	stephenfin	alex_xu: That was the reason we didn't do the implicit NUMA affinity, right?
15:03:46	mriedem	sure, but we don't necessarily have a dest at the point of that check
15:03:51	mriedem	yes
15:04:08	artom	So I don't want to say "you enabled the workaround, things will explode"
15:04:14	mriedem	artom: i mentioned this here https://review.opendev.org/#/c/640021/48/nova/conductor/tasks/live_migrate.py@196
15:04:20	artom	More like "you enabled the workaround, things will explode if the source and dest are old"
15:04:25	artom	(explode == eggagerating)
15:04:28	luyao	stephenfin, stephenfin : i'm confused about 'implicit' numa
15:04:35	spatel	Help!! - I want to evacuate compute nodes because it has hardware issue, but all running vm using local-disk ( NOT shared disk ) can i do evacuate?
15:04:35	alex_xu	stephenfin: no, that isn't. The reason do implicit NUMA topology for instance just due to someone asking that usecase in the PTG
15:04:38	mriedem	artom: i don't think that's what dan suggested
15:04:49	mriedem	"Computes are too old to do the smart thing, workaround is enabled, doing that"
15:04:59	dansmith	spatel: see topic
15:05:13	alex_xu	stephenfin: the explicity NUMA topology for instance and without affinity is clear plan in the PTG
15:05:15	mriedem	you adjust the warning to say that computes are too old to do the supported method or whatever
15:05:23	dansmith	spatel: you might notice that we're a day before FF and there are multiple dev-related discussions going on in here
15:05:53	spatel	dansmith: sorry about that, you guys continue.
15:05:53	luyao	stephenfin,alex_xu: what is implicit NUMA affinity
15:05:58	stephenfin	alex_xu: It's sounding like the implications of that plan were not properly understood though. Certainly I didn't grasp them
15:07:01	alex_xu	luyao: I guess that is just stephenfin typing wrong word, actually mean implicity NUMA topology
15:07:19	alex_xu	stephenfin: ^ is that right? or you pointed to something I may not get
15:07:26	mriedem	artom: i left a suggestion on wording
15:07:28	stephenfin	luyao: Implicit NUMA affinity is what we do for hugepages and CPU pinning. If you use either of those features, your guest will have a one-node NUMA topology and it will be pinned to a host NUMA node
15:07:45	stephenfin	alex_xu: Yeah, an implicit NUMA topology
15:08:19	luyao	stephenfin: I thought implicit Alex said is the guest numa not bind to host numa
15:08:24	mriedem	stephenfin: oh i have a solution - drop the numa part but only support vpmems on ppc64 https://github.com/libvirt/libvirt/blob/master/src/qemu/qemu_domain.c#L11604-L11615 :)
15:08:32	mriedem	power to the rescue
15:08:47	stephenfin	fight the power
15:08:51	luyao	stephenfin, alex_xu: so you are discuss different things ?
15:09:15	mriedem	does tonyb have openshift working on power yet?
15:09:23	stephenfin	luyao: no, the disagreement is on the lack of pinning
15:09:45	artom	mriedem, https://review.opendev.org/#/c/640021/50/nova/conductor/tasks/live_migrate.py@200
15:10:40	stephenfin	the fact that for hugepages, the single guest NUMA cell is pinned to a host NUMA cell, but for VPMEM, the single guest NUMA cell floats across all host NUMA cells
15:10:48	stephenfin	*for hugepages and CPU pinning
15:11:19	alex_xu	stephenfin: yes, so you want the pinning, but is it ok without affinity for now?
15:11:25	stephenfin	bauzas: Think you could hit these two too? https://review.opendev.org/#/c/681060/ https://review.opendev.org/#/c/681061/
15:11:43	stephenfin	alex_xu: I'll take that as a middle ground, yes
15:12:04	bauzas	stephenfin: looking
15:12:15	alex_xu	stephenfin: so the only I need is fall into this branch https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1740, right
15:12:24	stephenfin	yup
15:12:28	alex_xu	cool
15:12:57	mriedem	artom: replied
15:13:02	mriedem	with anger
15:13:07	luyao	stephenfin: Does it make sense to do the numa pinning now without affinity.
15:13:39	sean-k-mooney	luyao: yes
15:13:47	stephenfin	luyao: It's definitely sub-optimal, I agree, but it's better than breaking our long-standing one guest NUMA node == one host NUMA node policy
15:14:01	sean-k-mooney	its not ideal but its not a bad thing
15:14:01	artom	mriedem, ok, there's something I genuinely don't get then
15:14:09	luyao	sean-k-mooney: Okay
15:14:14	stephenfin	you'll get inferior performance but from a user perspective, things behavior as expected
15:14:20	artom	Not trying to make you angry on purpose, for serious
15:14:32	stephenfin	*behave
15:14:59	artom	If the conductor's train, and the source is train, and the dest is train, but there are other stein computes in the cell
15:15:15	artom	We'll get past the min sevice version check
15:15:39	artom	get past = not return
15:15:49	artom	Meaning we check the value of the workaround
15:15:54	artom	If the workaround is disabled, that's that
15:16:22	artom	If the workaround is enabled, in *this specific case of dest and source being new*
15:16:27	artom	We'll get NUMA LM
15:17:47	mriedem	artom: even if the source is new and you checked that, the dest - which we might not have yet if the user didn't force the dest from the api and bypass the scheduler - might not be new, and we won't know in that code with the warning log message because it comes before we ask the scheduler for a dest
15:18:15	mriedem	so if we get lucky and the source host and selected dest host are new enough, yeah it might be fine
15:18:24	mriedem	but,
15:18:26	dansmith	mriedem: I don't think that's whathe's saying
15:18:29	mriedem	it probably won't be because if you have old computes,
15:18:42	mriedem	rpc is going to be pinned and we'll backlevel the new numa migrate_data object stuff
15:18:56	dansmith	this ^ is what he's saying
15:19:14	dansmith	so he just doesn't want to specifically say that the src or dst is too old.. right?
15:19:23	mriedem	we never asked him to say that
15:19:35	mriedem	we just said "say something about computes not being upgraded yet"
15:19:38	dansmith	I don't think he claimed we did
15:19:46	mriedem	who's on first?
15:19:55	dansmith	I think he just said he's trying to wordsmith a suitably generic message right?
15:20:03	mriedem	i provided one
15:20:16	artom	Well there's the logging on L196 as well - it all fits together
15:20:32	artom	Or I'm just overcomplicating everything?
15:20:50	dansmith	I don't see any logging on L 196
15:21:14	artom	You asked for it :)
15:21:40	mriedem	artom: i think dan just meant adjusting the msg object
15:21:40	dansmith	you mean the logging that would be on L196 if you updated?
15:21:53	mriedem	if the config is enabled, we log a warning

Earlier   Later