| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-16 | |||
| 18:45:44 | artom | mriedem, ah crap | |
| 18:45:46 | efried | cdent Yeah, there's that too. I'm not sure that's being accounted for. | |
| 18:45:50 | mriedem | artom: i'm fixing | |
| 18:45:51 | artom | I thought I checked for min libvirt version | |
| 18:45:54 | artom | Clearly I didn't :( | |
| 18:46:13 | artom | At least not far back enough | |
| 18:46:52 | artom | mriedem, what's the fix? Check for libvirt version in code? | |
| 18:47:25 | efried | cdent I think my main issue is that I don't think we have the semantic codified for the general case in any spec. | |
| 18:47:36 | cdent | efried: my understanding, although I don’t know if this is explicitly expressed in spec or code anywhere, is that if something sets itself up to sharing provider (which requires two things: being in an aggregate with some other stuff, having the MISC_SHARES_VIA_AGGREGATE trait), then it is obliged to only aggregate with something that is a root provider | |
| 18:47:42 | mriedem | artom: hasattr | |
| 18:48:06 | cdent | efried: that is, it is the domain of aggregation that helps avoid some of the problems | |
| 18:48:35 | artom | mriedem, ah. Would we have a way to test that in CI with an old libvirt version? | |
| 18:49:22 | mriedem | artom: in unit tests | |
| 18:49:27 | efried | cdent Is it possible for a compute host to have DISK_GB inventory *and* be in an aggregate with a shared storage RP? | |
| 18:49:36 | mriedem | efried: yes | |
| 18:49:46 | artom | mriedem, you mean with a fake python-libvirt? | |
| 18:50:07 | mriedem | efried: well, maybe :) | |
| 18:50:13 | mriedem | not sure actually | |
| 18:50:54 | efried | mriedem cdent So this is the scenario alex_xu and I discussed earlier. If I ask for storagey traits X and Y; and my local disk has X but not Y; and my shared storage has Y but not X, I *should* not see this compute host in the allocation candidates. | |
| 18:50:59 | cdent | efried, mriedem: that’s something we initialy said “no” and then later changed to “yes” and then later “actually not sure" | |
| 18:51:00 | efried | But I think as currently written, I will. | |
| 18:51:39 | cdent | efried: you are correct, you should not get the compute host | |
| 18:51:41 | efried | cdent mriedem Which probably means we should architect it in such a way that we don't lock ourselves into disallowing it. | |
| 18:51:44 | cdent | because it can’t satisfy itself | |
| 18:51:54 | cdent | nor can it satisfy with the help of an aggregate friend | |
| 18:52:21 | efried | cdent Right; but I think as https://review.openstack.org/#/c/479766/10/nova/objects/resource_provider.py is written, that *will* hit. | |
| 18:53:07 | efried | And once nested is in play, it gets even messier. Because we *are* allowing a certain amount of trait "propagation" within a tree. But only in some directions. | |
| 18:53:10 | cdent | I’ve been unable to fully decode the SQL in that stack, so have mostly just been waiting for some tests along those lines | |
| 18:54:08 | cdent | so what I would think is the right thing here, is making sure, since we already have the concept of shared within the object code, is that there are functional tests in db/test_resource_provider.py for the scenarios you’ve described | |
| 18:54:27 | cdent | and yeah, once nested is in play, it will get even more interesting | |
| 18:54:42 | bauzas | dansmith: thanks for efried's change, will look | |
| 18:54:49 | bauzas | mriedem: ack, will look | |
| 18:56:37 | efried | bauzas Note that there's still work to be done there. The change is kinda blowing up. Hope to have another rev today or tomorrow. | |
| 19:00:28 | openstackgerrit | Merged openstack/python-novaclient master: Update "The nova Shell Utility" in the user guide https://review.openstack.org/512224 | |
| 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 | cdent | [t NeX] | |
| 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: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 | |