| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-23 | |||
| 13:29:53 | efried | alex_xu Because at some point you were going to make changes to parse the 'required' query param to feed into your series. | |
| 13:30:00 | efried | alex_xu You haven't started that work yet, have you? | |
| 13:30:27 | alex_xu | efried: I have, https://review.openstack.org/#/c/479776/ but didn't update recently | |
| 13:31:07 | efried | alex_xu Okay, I'll read that one over. | |
| 13:31:22 | alex_xu | efried: we probably need to put the schema and parse code of 'required' as common | |
| 13:31:46 | alex_xu | efried: do you have any sample code for your get_by_filters_grouped | |
| 13:31:57 | alex_xu | I guess that is the hard part | |
| 13:32:21 | efried | alex_xu Right, my thought was that we might as well do it all at once. That is, make the API change from accepting just 'resources' to accepting 'resources[N]' + 'required[N]' in one microversion jump. | |
| 13:33:35 | alex_xu | efried: since microversion is micro, we shouldn't put too much in single version. but that should be ok | |
| 13:33:36 | efried | alex_xu No, I haven't written any of that yet. The easy part will be grabbing the candidates for a single numbered grouping. Because (per the spec) that's always coming from a single RP - I don't need to mess with aggregates or trees. | |
| 13:34:22 | efried | alex_xu Slightly harder will be combining that with the results from the unnumbered group. But even that shouldn't be *too* hard - we just need to match up the aggregates at that point. | |
| 13:35:22 | efried | alex_xu But I haven't done any serious SQL in ~15y, and never done any sqlalchemy, so I'm definitely going to be looking for guidance/help/mentoring there. | |
| 13:36:06 | alex_xu | efried: do you read the patch I just submitted https://review.openstack.org/#/q/topic:bp/add-trait-support-in-allocation-candidates+(status:open+OR+status:merged)? | |
| 13:36:34 | alex_xu | I created few objects to instead of currenlty single huge method | |
| 13:37:03 | efried | alex_xu I saw those come through, but haven't looked them over yet. But that's what prompted me to ping you. | |
| 13:37:28 | alex_xu | the AggregatedProviderSummary obj will be the one to indicate a set of providers which are in the same aggregate | |
| 13:37:55 | alex_xu | so I'm think whether we need other object to build a tree for nested resource provider | |
| 13:38:08 | efried | alex_xu That already exists: ProviderTree | |
| 13:38:22 | alex_xu | efried: ProviderTree is for nova, not for placement | |
| 13:38:31 | efried | oh, right. | |
| 13:38:58 | cdent_ | why do we need to represent the tree on the placement side? (not saying we don’t just want to clarify on why) | |
| 13:39:21 | efried | cdent_ Not strictly necessary, but sure does make it easier to conceptualize. | |
| 13:39:31 | efried | cdent_ For things like trait propagation | |
| 13:39:31 | cdent_ | “it”? | |
| 13:39:46 | cdent_ | I’m still not entirely clear on the reality of trait propagation | |
| 13:40:01 | cdent_ | especially in a world of a numbered resources | |
| 13:40:53 | mriedem | dansmith: i think the bottom alternate hosts change is ready https://review.openstack.org/#/c/486215/ | |
| 13:40:57 | mriedem | you and jay were +2 before | |
| 13:40:59 | alex_xu | emm...any hint about trait propagation? my brain becomes empty, not sure due to the time or not... | |
| 13:41:00 | efried | cdent_ There was an example in the spec. Possibly in review comments rather than in the actual text. And it may not have been a great example, at that. | |
| 13:41:34 | efried | alex_xu cdent_ It may have been something like this: | |
| 13:42:38 | efried | alex_xu cdent_ I have multi-port SR-IOV NICs associated with NUMA nodes. Each port is connected to a different physnet. | |
| 13:42:59 | dansmith | mriedem: ack, I wanted to be happy with the usage of the object before we merged the one that added it, but I guess that bottom one is separate | |
| 13:43:18 | efried | alex_xu cdent_ So my model looks like: compute => numa node => PF | |
| 13:44:46 | efried | alex_xu cdent_ The NUMA node has a trait indicating which node it is, and it holds the VCPU and MEMORY_MB inventory. The PF has a trait indicating which physnet it's on, and it holds the VF inventory. | |
| 13:45:33 | efried | alex_xu cdent_ But the logic still needs to be able to know that the PF belongs to the NUMA node associated with its parent RP. | |
| 13:45:50 | alex_xu | efried: the NUMA node needn't a ttrait to indicate, but nvm, let us continue your example | |
| 13:46:10 | alex_xu | efried: yea, probably | |
| 13:46:16 | efried | Perhaps NUMA is a bad example because you wouldn't be making a request for a specific NUMA node. This is more about affinity, which is for later. | |
| 13:46:39 | efried | Let me go see if I can dig up the example from the spec... | |
| 13:47:18 | alex_xu | efried: cdent_ I'm thinking we may need a obj just like this for shared rp https://review.openstack.org/#/c/514200/1/nova/objects/resource_provider.py@2552 to show the path for nested resource provider | |
| 13:47:32 | alex_xu | so I mean we needn't build a tree | |
| 13:47:53 | alex_xu | what we need a build a list of path for the condicates | |
| 13:48:08 | alex_xu | s/condicates/candidates | |
| 13:48:31 | alex_xu | what we need is that building a list of pathes for the candidates | |
| 13:48:44 | efried | You mean every possible "branch"? | |
| 13:48:53 | alex_xu | efried: yea, branch | |
| 13:49:29 | efried | In the general case that could be a big list, but in reality I suppose it would probably be manageable. | |
| 13:50:04 | alex_xu | shrink the scope of all the branches by the sql, then filter the final branch in the code | |
| 13:50:49 | alex_xu | if we want to done everything in the sql, that will be super complex sql just like current we have, and only Jay know how to write a such sql :) | |
| 13:50:57 | efried | alex_xu cdent_ Here's the example from the spec discussion: https://review.openstack.org/#/c/497713/6/specs/queens/approved/add-trait-support-in-allocation-candidates.rst@42 | |
| 13:53:37 | alex_xu | efried: I see | |
| 13:53:57 | alex_xu | not really propagation, but probably that is rule how we check each branch | |
| 13:54:04 | cdent_ | efried: yeah, I remember that. I think my issue isn’t so much with behavior as with terminology. | |
| 13:54:30 | efried | cdent_ We already spent a ton of emotional energy haggling over "propagate" vs "inherit". | |
| 13:54:36 | cdent_ | I agree that in the request example, you should get back a tree that includes the NUMA, and the compute node. that’s not quite the same as "hit" | |
| 13:55:20 | efried | cdent_ Well, in that context, "hit" means we actually got that candidate. As opposed to NOT getting that candidate because the NUMA RP doesn't have the CPU trait itself. | |
| 13:56:15 | cdent_ | there is a successful allocation candidate because numa is in a tree that has the trait | |
| 13:56:41 | cdent_ | which maybe is exactly the same thing as you are saying | |
| 13:57:02 | efried | cdent_ Yes, except not "in a tree". | |
| 13:57:22 | cdent_ | well, it is | |
| 13:57:24 | efried | cdent_ Has to be at or above the resource. | |
| 13:57:33 | efried | Yes, it's in the tree, but not *anywhere* in the tree. | |
| 13:57:42 | cdent_ | oh, yes “above" | |
| 13:57:49 | efried | e.g. if the "cousin" numa node had the trait, that wouldn't be acceptable. | |
| 13:58:27 | efried | Rewrite: if the "cousin" numa node had the trait (but the compute RP didn't), that wouldn't be acceptable. (We would get the cousin, in fact.) | |
| 13:58:31 | cdent_ | I think the thing I’m getting hung up on is that in my mind the code is doing a “look up to see if I’m good enough” | |
| 13:58:57 | cdent_ | but we keep talking about traits going down, and that just doesn’t say anything about the process | |
| 13:59:10 | cdent_ | but in the end I think we agree, so probably best I just let it lie | |
| 13:59:13 | efried | Where "good enough" includes both the notion of "has enough resources" and "has the right traits". And we're just talking about the second thing right now. | |
| 13:59:40 | cdent_ | because my mental model for how placement works is probably, uh, unique | |
| 14:00:16 | efried | I agree the process is where there be tygers, but we're not supposed to worry about how the SQL's gonna work :) | |
| 14:00:37 | cdent_ | except that’s the part that’s the hardest to understand :( | |
| 14:00:43 | edleafe | Scheduler subteam meeting starting now in #openstack-meeting-alt | |
| 14:01:19 | efried | cdent_ Totally agree. | |
| 14:02:09 | jaypipes | efried: not just the cousin. the brother/sister. | |
| 14:02:30 | efried | jaypipes Yes, right. | |
| 14:02:49 | efried | Or even second cousins once removed. | |
| 14:02:55 | stephenfin | artom: What do you mean by the third comment here? https://review.openstack.org/#/c/345399/29/nova/tests/unit/console/securityproxy/test_rfb.py | |
| 14:03:03 | efried | Basically anything that ain't self or direct ancestor | |
| 14:03:51 | cdent_ | yeah, I think we all agree on that aspect | |
| 14:06:05 | dtantsur | efried, cdent, I'll send the doodle to the ML, will cc you, jaypipes and johnthetubaguy | |
| 14:06:17 | cdent | thanks dtantsur | |
| 14:06:20 | efried | dtantsur Thanks! | |
| 14:06:40 | johnthetubaguy | efried: should I re-read the scroll back? | |
| 14:06:52 | efried | johnthetubaguy No, just the ML. | |
| 14:06:56 | johnthetubaguy | OK | |
| 14:16:23 | artom | stephenfin, dammit, your different upstream/downstream nicks confuse me | |
| 14:16:43 | artom | stephenfin, to answer your question, \x04blah means, IIRC, reason-length of 4 bytes, then 'blah' is the reason length | |
| 14:17:31 | artom | I was suggesting we test reason-lengths different to 4, since both test_fail and test_fail_server_message have the same content | |
| 14:17:43 | artom | ... with a reason-length of 4 | |
| 14:18:48 | artom | Or, have a unit test just for _make_var_str, which is what transforms 'blah' into the '\x04blah' that you send over the wire | |
| 14:19:03 | artom | And test it with at least 2 different reason-lengths | |
| 14:19:33 | artom | Obviously not a massive deal | |
| 14:19:42 | artom | Just seems more thorough :) | |
| 14:20:15 | artom | Err, in my second line, 'blah' is the reason, not reason length | |
| 14:20:44 | efried | johnthetubaguy In the scheduler meeting (#openstack-meeting-alt) there's about to be a discussion of whether you can remove a trait on a RP that has allocations - your interest is via https://review.openstack.org/#/c/504531/8/specs/approved/node-traits.rst@86 | |
| 14:36:01 | alex_xu | jaypipes: I try to refactor the alllocaion candidates in implement the traits support https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/add-trait-support-in-allocation-candidates, appreciate if you can give some feedback on the direction | |
| 14:39:17 | openstackgerrit | Jan Zerebecki proposed openstack/nova master: Only log not correcting allocation once per period https://review.openstack.org/508262 | |
| 14:40:57 | johnthetubaguy | efried: did you cover that? | |