| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-04 | |||
| 17:11:14 | claudiub | yep | |
| 17:11:39 | sean-k-mooney | this is has alwasy been the case since numa was firsts added | |
| 17:12:01 | sean-k-mooney | the specific behavior is implemation defiend and is not garenteed by the api | |
| 17:12:02 | artom | stephenfin, D: how did you not jump on https://review.opendev.org/c/openstack/nova/+/774240/12/nova/virt/libvirt/driver.py#697 with a -2? | |
| 17:12:04 | artom | ;) | |
| 17:12:35 | sean-k-mooney | so it can be modified but the current beahivor optimises for being able to spawn large vms | |
| 17:12:56 | sean-k-mooney | if we balance the vms between numa ndoes it will improve performance in genreal but pessimise spawning large vms | |
| 17:13:11 | sean-k-mooney | so there is a tradeoff between packeing and spreading/blancing | |
| 17:13:17 | claudiub | i agree there, and I am aware of that. :) | |
| 17:13:36 | sean-k-mooney | yep so this is why we said this cant be change in a bug and need a spec | |
| 17:13:44 | sean-k-mooney | which means upstream at least not backportable | |
| 17:14:03 | sean-k-mooney | downstream we likely would backport it but not chagne the behavior by default | |
| 17:14:13 | stephenfin | artom: My yoga guy said I needed to be more chill about these things | |
| 17:14:17 | stephenfin | I'll fire him in the morning | |
| 17:14:20 | sean-k-mooney | upstream if accpetd i would like to change the default but in the cycle after its added | |
| 17:14:43 | stephenfin | <sean-k-mooney> so it can be modified but the current beahivor optimises for being able to spawn large vms | |
| 17:14:51 | stephenfin | are you talking about unpinned instances? | |
| 17:15:01 | sean-k-mooney | stephenfin: no all numa instances | |
| 17:15:16 | sean-k-mooney | stephenfin: by packing numa nodes before moving on | |
| 17:15:22 | sean-k-mooney | we keep the rest free | |
| 17:15:33 | sean-k-mooney | so vms that need all teh ram or cpus on a numa node can boot | |
| 17:15:44 | stephenfin | ah, yeah I don't think that's a good argument for the unpinned case | |
| 17:15:45 | sean-k-mooney | if we spread then that makes large vms less likely to fit | |
| 17:15:56 | stephenfin | spreading makes sense there IMO | |
| 17:15:59 | sean-k-mooney | unpinned float so ya not an issue | |
| 17:16:03 | stephenfin | yup | |
| 17:16:16 | claudiub | in our scenario, large vms is not a concern, since we have pretty large hosts, so in our case, it would work better for a spread-out approach. But indeed, not everyone is the same. Could this be a config option then? | |
| 17:16:20 | stephenfin | if you don't spread, you'll basically never end up on node 1, as claudiub is seeing | |
| 17:16:26 | stephenfin | or node N > 0 | |
| 17:16:43 | sean-k-mooney | claudiub: yep it could be a config option or it coudl be done via aggreate metadata | |
| 17:17:00 | stephenfin | I don't think that's needed or unpinned | |
| 17:17:03 | stephenfin | *for | |
| 17:17:27 | sean-k-mooney | stephenfin: well this is needed for all numa guests pinned or unpinned | |
| 17:17:33 | claudiub | hmm, aggregate metadata also sounds interesting. it could have best of both worlds | |
| 17:17:35 | sean-k-mooney | but its not needed for non numa instnaces | |
| 17:17:56 | sean-k-mooney | claudiub: my concern is if we codify this as a feature we need to supprot it with placment in the future | |
| 17:17:58 | stephenfin | pinned guests can already use NUMA nodes > 0 | |
| 17:18:16 | sean-k-mooney | claudiub: we can do that but that means we need to do the same behavior by sorting the allocation candiates | |
| 17:18:48 | sean-k-mooney | yep they can but that not really the issue | |
| 17:19:03 | claudiub | hm, I am a bit outside the loop with the placement api, but doesn't the NUMAPlacementFilter also use the numa_fit_instance_to_host function? | |
| 17:19:30 | claudiub | or whatever the fitler name was. :) | |
| 17:19:30 | sean-k-mooney | claudiub: yes currently we are not usign placmnet for numa and wont be for a few release | |
| 17:19:41 | claudiub | oh ok, gotcha. | |
| 17:19:46 | stephenfin | can we back up and say why any of this needs a aggregate metadata filter | |
| 17:19:56 | stephenfin | we're not trying to change stack/spread behavior for pinned instances, right? | |
| 17:20:00 | sean-k-mooney | claudiub: the concern is the more featers the filter has the more we need to port to the plamcent version | |
| 17:20:10 | sean-k-mooney | stephenfin: no new aggreate filter | |
| 17:20:18 | stephenfin | only unpinned NUMA instances, because those are all landing on NUMA node 0 | |
| 17:20:30 | stephenfin | sorry, an aggregate metadata key | |
| 17:20:40 | stephenfin | <sean-k-mooney> claudiub: yep it could be a config option or it coudl be done via aggreate metadata | |
| 17:20:40 | sean-k-mooney | stephenfin: nope hugepages and all other numa instnace land there too | |
| 17:20:42 | stephenfin | ^ that | |
| 17:21:14 | stephenfin | a page with hugepages and no pinning *is* an unpinned NUMA instance | |
| 17:21:16 | sean-k-mooney | stephenfin: if we do it per host it has to be accounted for on live migration and we have the same proble we had wtih PCPUs and hyperthreading | |
| 17:21:41 | claudiub | Hm, I'm wondering why the current implementation is: "Hey HostState, I'm a request spec, pls fit me", rather than: "Hey HostState, Placement told me to sit in your X numa node." | |
| 17:21:41 | stephenfin | *host | |
| 17:21:41 | stephenfin | if we do what per hose? | |
| 17:21:54 | sean-k-mooney | stephenfin: allow packing vs spreading | |
| 17:22:10 | stephenfin | I'm not suggesting making it configurable at all | |
| 17:22:28 | sean-k-mooney | well people objected ot hardcoding spreading | |
| 17:22:35 | stephenfin | the current packing behavior is a bug | |
| 17:22:43 | stephenfin | did they? Link? | |
| 17:22:53 | stephenfin | claudiub: Yes, it's the former | |
| 17:23:01 | sean-k-mooney | i filed https://bugs.launchpad.net/nova/+bug/1893121 and was told its a feature not a bug | |
| 17:23:02 | openstack | Launchpad bug 1893121 in OpenStack Compute (nova) "nova does not balance vm across numa node or prefer numa node with pci device when one is requested" [Undecided,Confirmed] - Assigned to sean mooney (sean-k-mooney) | |
| 17:23:35 | stephenfin | claudiub: placement gives us X VCPU inventory, but it has nothing to do with what actual host CPUs are used | |
| 17:23:35 | sean-k-mooney | stephenfin: then i brought it up in the ptg as a bug and was told it need a spec https://etherpad.opendev.org/p/nova-wallaby-ptg | |
| 17:23:43 | sean-k-mooney | stephenfin: line 710 | |
| 17:24:41 | stephenfin | claudiub: Put another way, placement says you may have 4 unpinned/pinned CPUs, and nova-compute says you may map to these specific hosts core(s). Placement doesn't track the specifics | |
| 17:24:56 | sean-k-mooney | stephenfin: if we alwasys want to spread thats simple. and thats what i wanted to do orginally | |
| 17:25:46 | stephenfin | sean-k-mooney: Yeah, that's never an RFE. I'm not sure how we came to that conclusion | |
| 17:26:28 | stephenfin | Packing all your instances onto one host NUMA node and leaving the others empty regardless of the number of instances created is a bug every day of the week :) | |
| 17:26:33 | sean-k-mooney | if we are ok treating this as a bug the i will try to work on it next cycle and backport it | |
| 17:26:49 | stephenfin | well it sound like claudiub might have time to work on it also | |
| 17:26:52 | stephenfin | which would be great :) | |
| 17:26:56 | gibi | stephenfin: what will happen when we model NUMA nodes in placement? I guess at that point placement will track how many PCPU belongs to which NUMA node. | |
| 17:27:13 | sean-k-mooney | gibi: we will need to sort the allocation candiates | |
| 17:27:15 | stephenfin | gibi: s/when/if/ ;) | |
| 17:27:22 | sean-k-mooney | to have the same behviaor | |
| 17:27:32 | stephenfin | yeah, what sean-k-mooney said | |
| 17:27:32 | sean-k-mooney | we should get mutiple allcoation candiate per host | |
| 17:27:57 | gibi | OK. I'm out of brain power but I feel that if we discussed it once and came to a conclusion that it is a feature then there might be complications | |
| 17:28:07 | sean-k-mooney | gibi: right now since the behavior is currently undefiend we can pretend the exsiting behavior is not a thing | |
| 17:28:49 | stephenfin | I say we work on the patch and then review | |
| 17:28:56 | stephenfin | complications should be evident by then | |
| 17:29:11 | sean-k-mooney | gibi: the concern was that it could cause large vms that previousl would have boot to fail | |
| 17:29:19 | stephenfin | and we can adjust accordingly | |
| 17:29:38 | gibi | so this will be somehow configurable or user requestable to pack or spread? | |
| 17:29:54 | sean-k-mooney | that was the reason we said i should write a spec | |
| 17:29:56 | stephenfin | sean-k-mooney: that seems like an acceptable compromise given the alternative | |
| 17:29:58 | sean-k-mooney | to debate that point | |
| 17:30:14 | sean-k-mooney | stephenfin: it not that we wont use the other numa nodes ever | |
| 17:30:24 | sean-k-mooney | we will if the vm does nto fit on the first node | |
| 17:30:53 | claudiub | btw, I had nodes with ~400 instances on a single NUMA node, and the other numa node was empty. :) | |
| 17:31:07 | stephenfin | sean-k-mooney: I don't think that will ever happen | |
| 17:31:12 | stephenfin | Yeah, what claudiub says | |
| 17:31:13 | sean-k-mooney | stephenfin: it does | |
| 17:31:26 | stephenfin | I'm almost certain we don't apply overcommit ratios correctly on a per-node basis | |
| 17:31:27 | sean-k-mooney | claudiub: you likely have misconfigured your flavors | |
| 17:31:59 | stephenfin | As above, I say we get a fix and functional test for this and then debate it there | |