| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-11 | |||
| 15:00:30 | bauzas | dansmith: heh, so true | |
| 15:00:38 | artom | It's tricky because... just because we found some old computes in the cell, doesn't mean the source and dest are old | |
| 15:00:38 | stephenfin | to be able to do NUMA affinity properly, we'll need to provide information about the VPMEM devices available in each node as part of the NUMATopology/NUMACell objects | |
| 15:00:41 | dansmith | sean-k-mooney: heh | |
| 15:00:42 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Skip querying resource request if no qos port https://review.opendev.org/681513 | |
| 15:00:46 | artom | So I want to word around that | |
| 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 | |