| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-23 | |||
| 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? | |
| 14:41:11 | efried | johnthetubaguy Not yet. | |
| 14:43:45 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Don't disregard cache mode for instance boot disks https://review.openstack.org/514339 | |
| 15:01:10 | cdent | jaypipes: on the link state thing, we don’t want to kill an entire compute service for one pf do we? | |
| 15:01:42 | jaypipes | alex_xu: yes, will try to review that whole series today. I'm doing new hire stuff and meetings today and tomorrow, though, so reviews will be slwo | |
| 15:01:44 | jaypipes | slow | |
| 15:02:08 | alex_xu | cdent: jaypipes edleafe efried, the trait spec said the provider summary of the allocation candidates API response includes all the traits in that rp, not only the required. For the resources in the response, the current implementation is only include the requested resources, is that we expected? or it is bug | |
| 15:02:18 | jaypipes | sean-k-mooney: and yes, I definitely saw you mention "hardware-defined software" on the ML. :) | |
| 15:02:25 | alex_xu | jaypipes: thanks | |
| 15:02:50 | cdent | alex_xu: sounds like bug | |
| 15:02:52 | jaypipes | alex_xu: probably a bug | |
| 15:02:57 | jaypipes | jinx :) | |
| 15:02:57 | efried | alex_xu In the provider summary or in the allocation_requests ? | |
| 15:03:01 | cdent | the provider summary should be “the provider" | |
| 15:03:06 | alex_xu | efried: yea | |
| 15:03:19 | dtantsur | cdent, jaypipes, johnthetubaguy, efried, https://doodle.com/poll/ts43k98kkvniv8uz (sorry, forgot to cc you) | |
| 15:03:29 | jaypipes | cdent: right, but there;'s a dict of resources in the provider summary showing remaining capacity for inventories that provider has. | |
| 15:03:49 | jaypipes | cdent: this was requested for weighers that want to sort based on remaining inventory capacity. | |
| 15:03:58 | alex_xu | ok...it is bug, then I need some change in my refactor patch | |
| 15:04:20 | efried | Why is it a problem if the remaining capacity is included for classes that weren't claimed? | |
| 15:04:23 | openstackgerrit | Merged openstack/nova master: Move shred to privsep. https://review.openstack.org/495537 | |
| 15:04:31 | cdent | jaypipes: i wasn’t meaning to be exclusive, rather that the entire provider should be reprsented (plus whatever else) | |
| 15:04:35 | openstackgerrit | Merged openstack/nova master: Move xend existence probes to privsep. https://review.openstack.org/495538 | |
| 15:05:21 | jaypipes | dtantsur: done | |
| 15:05:47 | jaypipes | efried: I don't think it is. I think it was probabl jjst an oversight | |
| 15:05:54 | jaypipes | god damnb this mac keyboard... :( | |
| 15:06:06 | efried | jaypipes Which part is an oversight? | |
| 15:06:16 | efried | jaypipes Including the extra RCs or excluding them? | |
| 15:06:29 | jaypipes | efried: that the provider summary for a provider only includes resources that were requested and not all resources the provider had | |
| 15:06:36 | efried | jaypipes Which way are we saying it's *supposed* to be? | |
| 15:07:00 | efried | That it should include *all* of them, I guess. I agree with that. | |
| 15:07:59 | cdent | efried: yes, all | |
| 15:08:04 | efried | ++ | |
| 15:08:25 | edleafe | efried: cdent: of what use do you see non-requested resources in the provider_summary? | |
| 15:08:30 | alex_xu | jinx means a person bring bad luck? | |