| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-26 | |||
| 16:07:36 | mriedem | efried: "Note that some people are opposed to typo-in-comment-or-docstring patches. It's a religious thing, I think. So don't be surprised if this gets the kibosh." heh | |
| 16:08:28 | mriedem | i'm not opposed to fixing documentation if it makes the documentation more clear, i am opposed to stat padding py pushing several 1-line spell check fixes across openstack at random | |
| 16:08:34 | mriedem | *by pushing | |
| 16:10:29 | efried | mriedem: I thought we didn't put enough stock in stats to make that the sole reason for rejecting things like this. IMO patches like this could be fast-approved more quickly than they can be squashed. And let the author have the stats - what difference does it really make? | |
| 16:10:36 | efried | Anyway, that's my 2c | |
| 16:10:49 | mriedem | it's not the sole reason | |
| 16:10:54 | stephenfin | I'd be more lenient. If it doesn't cause merge conflicts, it's good and could be conceivably fast approved. If someone's basing their employee reviews on Stackalytics, they're the fools | |
| 16:10:57 | mriedem | it's also noise | |
| 16:11:49 | efried | Meh, how much noise is it really? Are you worried about an explosion of trivial patches if you start approving these? | |
| 16:12:22 | mriedem | imo it encourages bad behavior | |
| 16:13:26 | cdent | FWIW I agree with efried | |
| 16:13:51 | cdent | the only thing that should matter in the end is the quality of the code | |
| 16:14:26 | stephenfin | cdent: With the caveat that it's trivial and doesn't cause merge conflicts. Functional changes still have to take priority | |
| 16:14:29 | efried | I agree there's a balance to be struck against reviewer time. In this case, it's eta/eta | |
| 16:14:59 | efried | (or whatever greek letter means "something really small") | |
| 16:15:11 | bauzas | gosh, I'm about to ragequit because of all the NUMA quirks we need to support | |
| 16:15:24 | stephenfin | bauzas: You're welcome :) | |
| 16:15:47 | bauzas | faking cpu sockets for licensing reasons => booooh | |
| 16:16:33 | bauzas | the question I wonder is, should https://docs.openstack.org/nova/latest/admin/cpu-topologies.html#customizing-instance-cpu-topologies be Placement-specific ? | |
| 16:16:36 | bauzas | my guts say no | |
| 16:16:42 | bauzas | stephenfin: jaypipes: thoughts on that ? | |
| 16:17:01 | stephenfin | What do you mean, "placement-specific"? | |
| 16:17:10 | bauzas | Resource classes and other things | |
| 16:17:32 | bauzas | IMHO, we should just provide the NUMA topology, find a node and period. | |
| 16:17:36 | jaypipes | bauzas: on a call... gimme a bit. | |
| 16:17:45 | stephenfin | bauzas: It doesn't affect what you need to claim so IMO no | |
| 16:17:55 | bauzas | yup, cool | |
| 16:17:57 | stephenfin | but jaypipes might have other ideas, once he's free | |
| 16:18:40 | openstackgerrit | Merged openstack/nova master: Fix api-ref: nova image-meta is deprecated from 2.39 https://review.openstack.org/554813 | |
| 16:22:43 | bauzas | efried: question, can I ask for both a resource query on a root node *and* a child RP ? | |
| 16:23:34 | efried | bauzas: The only place you can ask for resources that span RPs is in the un-numbered request group. | |
| 16:23:40 | bauzas | ideally, I'd love to see something like MEMORY_MB on both the root RP *and* the NUMA node | |
| 16:24:01 | bauzas | efried: can you please explain further ? | |
| 16:24:12 | efried | bauzas: Are those separate blocks of memory? | |
| 16:24:17 | bauzas | I dunno yet | |
| 16:24:21 | bauzas | (tbh) | |
| 16:24:39 | bauzas | I'm just hardly trying to not lock all the world | |
| 16:25:03 | efried | IMO we should not try to do the thing where we have e.g. 2048MB of memory in each NUMA node and then represent 4096MB on the compute node which isn't really there. | |
| 16:25:05 | bauzas | like I'd like to provide a mechanism to select a NUMA node but huge pages could still be checked by the virt driver | |
| 16:26:02 | bauzas | efried: I got your point, my problem is more about trying hard to not draw a house of cards | |
| 16:26:15 | efried | bauzas: is there a fixed ratio between memory MB and number of huge pages? | |
| 16:26:32 | bauzas | efried: the solution is simple in my mind | |
| 16:26:42 | bauzas | that's all about traits and step_size | |
| 16:26:45 | bauzas | but | |
| 16:26:59 | bauzas | if we go decide to provide a tree of NUMA nodes | |
| 16:27:35 | bauzas | but we don't support yet hugepages, then we have a problem because flavors asking for hugepages would get NoValidHosts | |
| 16:28:01 | bauzas | we somehow still need to accept compute nodes to report their memory the old way | |
| 16:28:09 | efried | what's a hugepage? | |
| 16:28:17 | efried | (use little words) | |
| 16:28:26 | bauzas | https://docs.openstack.org/nova/latest/admin/huge-pages.html | |
| 16:28:39 | bauzas | it's a defined memory page size | |
| 16:28:48 | bauzas | but that's just an example | |
| 16:28:57 | bauzas | for the moment, placement only checks the total memory of the host, right? | |
| 16:29:12 | bauzas | then the scheduler filter does magic things to find a destination | |
| 16:29:40 | efried | Well, yeah, so this is kind of in line with what we need to do wrt bandwidth. | |
| 16:29:52 | efried | We represent the number of huge pages as inventory alongside the MEMORY_MB inventory. | |
| 16:29:53 | efried | but | |
| 16:30:12 | efried | if the consumer is going to include hugepages as part of their request, they *always* need to do so. | |
| 16:30:27 | bauzas | so, my concern is that if I'm beginning to shard the memory between NUMA nodes, then it requires the flavors to be updated to explicitly ask for a NUMA node, whereas huge pages are totally NUMA unrelated | |
| 16:30:59 | efried | because what we can't (or shouldn't) do is try to convert a request for MEMORY_MB:4096 into MEMORY_MB:4096,HUGEPAGES:4 (or whatever) | |
| 16:31:26 | bauzas | efried: I'm not trying to design now how to make placement queries for hugepages | |
| 16:31:29 | efried | bauzas: But does a huge page come from the same place as a MEMORY_MB or is it a separate thing? | |
| 16:31:46 | bauzas | efried: what I'm trying is to make sure we keep a compatible behaviour for the existing feature | |
| 16:32:05 | bauzas | others, later, will try to solve that design and use placement resources for that | |
| 16:32:13 | bauzas | like the PCPU spec | |
| 16:33:06 | efried | bauzas: okay, maybe we back up and I just answer your original question :) | |
| 16:33:06 | bauzas | but again, what I want is to make sure that if operators enable reporting of NUMA nodes using NRPs, then it can still be possible to use hugepages flavors for finding a destination | |
| 16:34:51 | efried | You can specify multiple resources of different classes in a request group. A numbered request group will get *all* of those resources from the *one* resource provider. The un-numbered request group will get the resources from any provider in a tree or associated sharing providers. However, even in the latter case, all resources of a specific resource *class* will still come from a single provider. | |
| 16:39:51 | bauzas | efried: I see, thanks | |
| 16:40:07 | bauzas | so that could work | |
| 16:40:59 | bauzas | if I'm providing a NUMA topology through nested RPs, placement will give me a resource provider that supports that memory | |
| 16:42:17 | bauzas | efried: from a scheduler perspective, when it finds a nested resource provider as a destination when calling Placement API, I guess it uses the root RP for passing it down to the filters ? | |
| 16:43:42 | efried | bauzas: We haven't fully closed the switch on that yet, but yes, even if zero resource comes from the root RP, it'll still be the thing used as the "destination host". Not sure if that's a full answer to your question. Because filtering might need more info than that. | |
| 16:43:58 | efried | I'm guessing the entire allocation_request will need to be considered for some filters. | |
| 16:44:23 | bauzas | efried: that's the problem I see with NUMA filter | |
| 16:44:59 | bauzas | efried: because say placement finds a NUMA node, then it will return the child to the scheduler on a classic call | |
| 16:45:13 | bauzas | eg. a regular flavor | |
| 16:45:22 | bauzas | so we need to pass down the root RP | |
| 16:45:43 | bauzas | but then, the NUMA filter could try to find another NUMA node instead of using the one allocated | |
| 16:46:13 | efried | bauzas: Correlating the allocation_request with the provider_summary ought to allow you to figure out which NUMA node the resources were allocated from. | |
| 16:46:31 | efried | But yeah, without further invention, only the virt driver will know which RP UUID corresponds to which NUMA node. | |
| 16:47:10 | bauzas | that's not really the problme | |
| 16:47:16 | efried | ...which is kind of appropriate, because "identifying a NUMA node" is a virt-specific thing. | |
| 16:47:26 | efried | I.e. libvirt is gonna do it a different way than hyperv or whatever. | |
| 16:48:12 | bauzas | the problem is, say you ask for 2GB of memory within a NUMA node, then placement gives you host A with 2 NUMA nodes but only one NUMA node for host B | |
| 16:48:24 | bauzas | because the other NUMA node of host B is full | |
| 16:48:45 | bauzas | then, we need to pass to the scheduler filters the root RP | |
| 16:49:10 | bauzas | in theory, when it goes on NUMA filter for host B, it could consider the second NUMA node for host B as legit | |
| 16:49:17 | bauzas | there be dragons | |
| 16:50:10 | efried | how could it? | |
| 16:50:27 | efried | There's no candidate with allocations in that second NUMA node on host B. | |
| 16:50:36 | sean-k-mooney | bauzas: there might be dragons but the host state object for host b should also know that the second numa node is fully used and ignore it | |
| 16:51:14 | sean-k-mooney | bauzas: what is an issue if both numa nodes are valid and the filter chooses the other one form placement | |
| 16:51:49 | sean-k-mooney | e.g. placement decremetes the inventor that corresponds to node 0 but the numa topology filter decrements node 1 | |
| 16:51:58 | bauzas | okay, then I'm maybe overthinking | |
| 16:52:38 | sean-k-mooney | bauzas: there is an edge case here but its for host A with 2 NUMA not host B with 1 | |
| 16:53:52 | sean-k-mooney | for host b the resouce tracker will have updted the numatoplogy blob to also show the second numa nodes as full but in the case of host A both are valid from its point of view | |
| 16:55:02 | bauzas | I guess my fears are coming from the fact we litterally try to draw something out of nowhere, and without good testing for making sure we don't trample folks | |
| 16:55:14 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053 | |
| 16:55:28 | bauzas | if I was able to just test what I write, I wouldn't be trying to consider all the edge cases | |