| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-16 | |||
| 19:06:01 | openstackgerrit | Merged openstack/nova master: doc: Rework man pages https://review.openstack.org/502105 | |
| 19:10:02 | edleafe | efried: (reading back) so is the issue that the specified traits are not tied to a particular RP? | |
| 19:10:30 | efried | edleafe Yes, that's one aspect of it. | |
| 19:11:09 | efried | edleafe I'm comfortable with the semantics when we're just talking about nested RPs, but I think we have some holes (design points we haven't considered) when aggregates are put in the mix. | |
| 19:11:20 | edleafe | efried: agreed | |
| 19:11:53 | edleafe | efried: just wanted to make sure I followed what the problem was | |
| 19:12:43 | efried | edleafe cdent One suggestion would be to force the aggregate inventory+traits to be specified in its own numbered group (per the 'granular' spec). | |
| 19:12:49 | efried | Then we could greatly simplify the code. | |
| 19:13:10 | efried | Because we could search aggregates separately. | |
| 19:14:17 | efried | The thing I'm still unclear on is how much of a code commitment we've made to aggregates already. Are we reversing/breaking/removing "existing" function if we take aggregates completely out of the picture for Queens? | |
| 19:14:30 | efried | Given that we've said we're not doing shared RPs in Queens. | |
| 19:14:40 | cdent | efried: that kind of busts the point of aggregates, if you’re looking at things from the original shared disk need | |
| 19:14:47 | cdent | efried: as far as I can tell, no, nothing breaks | |
| 19:14:54 | cdent | the RT side keeps a map that it doesn’t use | |
| 19:15:08 | efried | cdent Okay, talk me through the "original shared disk need". | |
| 19:15:18 | cdent | “that kind of busts” -> where “that” is searching for aggregates separately | |
| 19:15:20 | efried | I mean, I understand the scenario. | |
| 19:15:36 | efried | scenario / use case | |
| 19:16:02 | cdent | the original shared disk need is: give me anything that can satisfying “I want some vcpu, disk and ram, doesn’t matter if some of it comes from a shared resource that is not the compute node and oh by the way, I’d like that disk to be SSD” | |
| 19:16:06 | openstackgerrit | Merged openstack/nova stable/ocata: Test InstanceNotFound handling in 'nova usage' https://review.openstack.org/482219 | |
| 19:16:31 | edleafe | efried: one idea had been to make traits on resources (e.g., per resource class). That was roundly rejected. | |
| 19:16:51 | edleafe | efried: breaking up traits by numbers seems like a variation of that | |
| 19:17:11 | efried | edleafe Ah, no, the traits are still on RPs. | |
| 19:17:22 | cdent | I’m unclear on the exact problem that traits by numbers is solving? | |
| 19:17:36 | edleafe | efried: I know, but the problem was the complexity, not the location | |
| 19:19:00 | dansmith | mriedem: these are ready for a re-look I think: https://review.openstack.org/#/c/506420 | |
| 19:19:09 | dansmith | fixed up based on the change I slid under both, and passing zuul | |
| 19:19:28 | mriedem | ok, still working on this ocata snafu | |
| 19:19:41 | efried | edleafe cdent Okay, expressing just this problem with aggregates... | |
| 19:20:50 | efried | Let's say hypothetically that I have a compute host with local disk that's capable of RAID5. And I have a shared storage provider whose storage is SSD. | |
| 19:21:35 | efried | Pre-nested, and pre-numbered-groups, but post-traits, you could only ask for inventory(VCPU, MEMORY_MB, DISK_GB) and traits(RAID5, SSD). | |
| 19:21:43 | edleafe | If you requests traits of RAID5 and SSD, that host should not match | |
| 19:21:46 | efried | Agreed. | |
| 19:22:01 | efried | edleafe But how would that work? | |
| 19:22:45 | efried | As currently architected, it *will* match that host. Cause it can't tell the difference between your CPU trait and your storage trait. | |
| 19:23:12 | edleafe | well, those are both storage traits, no? | |
| 19:23:19 | efried | So to make it clearer: | |
| 19:23:59 | efried | edleafe You ask for inventory(VCPU, MEMORY_MB, DISK_GB) and traits(RAID5, SSD, HW_CPU_X86_3DNOW) | |
| 19:24:40 | efried | edleafe What part of the architecture knows that it's okay for HW_CPU_X86_3DNOW to be split up from SSD, but it's not okay for RAID5 to be split up from SSD? | |
| 19:25:27 | edleafe | efried: for that you'll need jay to explain the big hairy sql | |
| 19:25:39 | edleafe | efried: fwiw, I don't know that it can | |
| 19:25:48 | efried | cdent So here's where numbered would come into play. I could ask for inventory1(VCPU, MEMORY_MB)&traits1(HW_CPU_X86_3DNOW) and inventory2(DISK_GB)&traits2(RAID5, SSD) | |
| 19:25:58 | efried | edleafe I believe it can't. There's no possible way it could. | |
| 19:26:25 | cdent | efried: I still don’t see the problem in the example you’ve given (non-numbered): | |
| 19:26:39 | efried | cdent Because it'll match that host+shared that it shouldn't. | |
| 19:26:48 | purplerbot | <efried> edleafe You ask for inventory(VCPU, MEMORY_MB, DISK_GB) and traits(RAID5, SSD, HW_CPU_X86_3DNOW) [2017-10-16 19:23:59.885269] [n NeX] | |
| 19:26:48 | cdent | [t NeX] | |
| 19:26:58 | edleafe | efried: if that's the case, then shared storage can't work, because shared storage would never have CPU traits | |
| 19:27:17 | cdent | right now that should not match, and you don’t want it to match | |
| 19:27:30 | efried | cdent Agree you don't want it to. But it *will*. | |
| 19:27:49 | cdent | efried: is that because of broken code or broken modelling, that’s what I’m trying to get at | |
| 19:28:44 | cdent | you should only get a trait if it’s resource provider is also providing inventory | |
| 19:28:50 | cdent | if that ^ statement is true | |
| 19:28:54 | efried | cdent Well, if we put a stake in the ground and say you can't possibly ever have a compute host with local storage associated ("aggregated") with a shared storage provider, then the scenario doesn't arise. | |
| 19:29:02 | cdent | and if you can’t split resource classes | |
| 19:29:05 | cdent | then no problem | |
| 19:29:50 | cdent | efried: let’s try to make this even more concrete, I’ll EOL when I’m done expressing the example | |
| 19:29:56 | cdent | two resource providers: | |
| 19:30:52 | cdent | compute_node1: inventory of VCPU 24, DISK_GB 1024, MEMORY_MB 1024, traits RAID5, HW_CPU_X86_3DNOW | |
| 19:31:36 | cdent | shared_diskA: inventory of DISK_GB 1024, traits RAID5, SSD, MISC_SHARES_VIA… | |
| 19:31:48 | efried | no, take RAID5 off of shared_diskA | |
| 19:32:03 | cdent | let me carry on as is | |
| 19:32:05 | efried | otherwise you match and it's not a problem. | |
| 19:32:06 | efried | okay. | |
| 19:32:10 | cdent | and then we’ll do that scenario | |
| 19:32:34 | cdent | those two are in an aggregate | |
| 19:33:39 | efried | GET /allocation_candidates?resources=VCPU:2,MEMORY_MB:512,DISK_GB:2&required=HW_CPU_X86_3DNOW,RAID5,SSD | |
| 19:33:50 | cdent | yes | |
| 19:34:16 | efried | You'll get { compute_node1: { VCPU: 2, MEMORY_MB: 512 }, shared_diskA: { DISK_GB: 2 } }. Cool. No problem. | |
| 19:34:31 | cdent | okay | |
| 19:34:43 | cdent | we agree that’s the design, right? | |
| 19:34:47 | efried | yup | |
| 19:34:51 | cdent | k, now | |
| 19:35:16 | cdent | shared diskA loses the RAID5 trait, but compute_node1 keeps it | |
| 19:35:23 | cdent | same request | |
| 19:35:33 | efried | Per the existing design, you will get the same result. | |
| 19:35:34 | cdent | the expected result, as designed, is nothi8ng | |
| 19:35:41 | efried | Correct. | |
| 19:35:57 | efried | sorry, "as designed" - we don't have a design point for this. | |
| 19:36:04 | efried | "as conceived", the expected result is nothing. | |
| 19:36:14 | cdent | yes, better choice of word | |
| 19:36:16 | efried | "as designed without having thought about this scenario" what you'll get is actually the same result as above. | |
| 19:36:33 | cdent | you have a test that shows that? | |
| 19:36:35 | efried | Because "as designed" we didn't think about this kind of split-trait scenario. | |
| 19:37:00 | efried | Well, no, because we haven't actually implemented traits on RPs yet. That's what alex_xu's change set is doing. | |
| 19:37:16 | efried | Which I suppose means there should be no harm in keeping that bit out of play for now? | |
| 19:37:41 | efried | ( ref: alex_xu's change set: https://review.openstack.org/#/c/479766/10/nova/objects/resource_provider.py ) | |
| 19:38:05 | cdent | a) there’s no harm keeping it out, but b) it’s the same thing I said before, we should write a test sooner than later that models the scenario you describe | |
| 19:38:29 | efried | cdent Yes, good idea. | |
| 19:38:31 | cdent | becuase as you talk about it, you make it sound like you think it expected to work. it is not | |
| 19:38:56 | cdent | if the second scenario is returing a result there is a bug | |
| 19:39:26 | efried | cdent Where should that test go? Cause without the above change, the framework isn't there. And if we keep aggregates out of the above change, same. | |
| 19:39:34 | cdent | and even if there is no code for it yet, we’ve got a test case prepared (at least mentally) for when there is | |
| 19:40:08 | cdent | isn’t there shared/aggregate code already pre-dating alex’s change? | |
| 19:40:18 | cdent | in objects/resource_provider.py | |
| 19:40:35 | efried | That's what I don't really know. | |
| 19:40:46 | efried | His change was really my first exposure to shared/aggregate code. | |
| 19:40:59 | cdent | one sec | |
| 19:42:59 | cdent | efried: https://review.openstack.org/#/c/460798/ introduced getting rps that share via misc | |
| 19:43:29 | efried | gaah, -1 for commit message width! | |