| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-04 | |||
| 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 | |
| 17:32:00 | sean-k-mooney | stephenfin: we dont but i think claudiub is missing hw:mem_page_size | |
| 17:32:07 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Replace blind retry with libvirt event waiting in detach https://review.opendev.org/c/openstack/nova/+/770246 | |
| 17:32:34 | gibi | I rest my case (at least for today) | |
| 17:32:39 | sean-k-mooney | stephenfin: we said we should start default that to hw:mem_page_size=any for all numa instnaces | |
| 17:32:40 | claudiub | hm, I only had the "hw:numa_nodes=1" extra_spec | |
| 17:32:40 | gibi | see you tomorrow | |
| 17:32:41 | lyarwood | gibi: awesome, I'll queue that up in the morning :) | |
| 17:32:46 | stephenfin | sean-k-mooney: that's a legit request | |
| 17:32:49 | sean-k-mooney | claudiub: yep that incorect | |
| 17:32:49 | lyarwood | gibi: \o | |
| 17:33:13 | gibi | lyarwood: note that it is still not all your comment fixed, but I will add one more separate patch to fix thoes | |
| 17:33:14 | sean-k-mooney | claudiub: you have enbaled numa node but not enable either numa aware cpu tracking or memory tracking | |
| 17:34:10 | sean-k-mooney | if you set hw:numa_nodes=1 and dont set hw:mem_page_size to any valid value its not correct | |
| 17:34:41 | stephenfin | that's a bit strong. It's fine if you don't have hugepages on the host | |
| 17:34:51 | stephenfin | or other things taking memory that you haven't accounted for | |
| 17:34:53 | sean-k-mooney | stephenfin: i did not say use hugepages | |
| 17:35:06 | stephenfin | setting hw:mem_page_size disables memory overcommit | |
| 17:35:08 | sean-k-mooney | stephenfin: i said set hw:mem_page_size to any value | |
| 17:35:26 | sean-k-mooney | stephenfin: yep you cant use over commit with numa properly | |
| 17:36:11 | sean-k-mooney | stephenfin: claudiub look at https://etherpad.opendev.org/p/nova-wallaby-ptg line 712 | |
| 17:36:49 | sean-k-mooney | i explained why this is needed | |
| 17:37:18 | stephenfin | that's a veritable hammer of a solution | |
| 17:37:26 | stephenfin | we could do proper tracking without that | |
| 17:37:32 | claudiub | ouch. didn't know about that | |
| 17:37:44 | sean-k-mooney | stephenfin: we cant we tried many times | |
| 17:38:38 | sean-k-mooney | stephenfin: the only time oversubsript can be used with numa guest is if hw:numa_nodes = the number of numa nodes on the host | |
| 17:38:39 | stephenfin | I appreciate it's complicated because we need to marry the non-NUMA model with the NUMA model | |
| 17:39:01 | stephenfin | that's just because we don't do proper oversubscription modelling on a per-node basis | |
| 17:39:24 | sean-k-mooney | we do proper page tracking on a per numa basis | |
| 17:39:37 | sean-k-mooney | and that woudl be used if we did hw:mem_page_size=small | |