Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-26
15:30:41 kashyap That said ... happy to snip it, and add it to a separate blog post or something. I'm all for brevity with clarity.
15:31:01 dansmith kashyap: ask someone else, but IMHO, it's about 500% too wordy
15:31:22 kashyap dansmith: Folks on #openstack-release said it reads very well, FWIW. smcginnis and dhellmann reviewed it
15:31:46 dansmith kashyap: cool, but I think it's too much
15:32:23 kashyap dansmith: I can cut it down. But I'd rather want to definitely retain the info about what an Operator would do with each of the 3 modes
15:32:34 kashyap It _certainly_ makes sense. As it's a completely valid question that will come up.
15:32:43 kashyap (As not everyone dwells on it.)
15:32:54 dansmith kashyap: that's fine, but I'll still be -1 on it
15:33:43 kashyap I'd rather first get someone else's view too in Nova.
15:34:36 dansmith kashyap: didn't I say ask someone else? :)
15:34:52 kashyap Sure :-)
15:35:46 dansmith efried: are you looking for me to add aggregates to ResourceRequest, or just to slap it into the result of resources_from_request_spec? because it seems like that function does some weirdness where it requests some resource group and then jams extra resources into the result
15:36:06 dansmith efried: this specifically: https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L334
15:36:58 dansmith I guess it's returning some internal object and then jamming the resource request in there
15:37:02 efried dansmith: That bit is for special handling of the "default resource classes". Wouldn't think aggregates would play in there.
15:37:19 dansmith efried: yeah, I'm trying to figure out where aggregates go and reading that is confusing to me
15:37:33 efried dansmith: Anyway, yeah, I was looking for ResourceRequest to get an agg field (similar to the traits field) which would be populated based on the request spec.
15:37:37 dansmith efried: you want an setter method on ResourceRequest that takes aggregates?
15:37:38 efried ...using the code you already done wrote.
15:37:50 dansmith or can I just set res_req.aggregates = []
15:37:52 dansmith ?
15:38:30 dansmith like, resources_from_request_spec() seems like it should be a classmethod on ResourceRequest, but it's not so I'm trying to figure out how much of "friends" they are
15:38:52 efried dansmith: um, looks like I already have member_of on RequestGroup .
15:39:13 dansmith okay and I don't really get the RequestGroup thing
15:39:24 dansmith and that comes out of placement,
15:40:08 dansmith so in that case, I _would_ actually do aggregates like the extra resources are getting jammed in there?
15:40:18 dansmith res_req.get_request_group(None).member_of = aggregates ?
15:40:25 efried yeah
15:40:33 dansmith mkay
15:40:42 dansmith I don't really get what this is doing, but.. as you wish
15:40:52 efried dansmith: It's the framework for granular.
15:41:04 dansmith ah, and that's what ident is then.. okay
15:41:37 efried dansmith: Puts in one place the parsing of the extra specs into the RequestGroups which will feed into the placement call.
15:42:12 dansmith efried: so, RequestGroup doesn't have much schema, so do you want me to:
15:42:21 dansmith grp.member_of = ['foo,bar', 'baz']
15:42:21 dansmith or
15:42:37 dansmith grp.member_of = [('foo','bar'), ('baz',)]
15:42:38 dansmith ?
15:42:57 efried dansmith: Guess it depends on how that spec shakes out :P
15:43:02 jaypipes gibi: hey, sorry, went to get something to eat. I'll respond on the spec.
15:43:16 openstackgerrit Matthew Booth proposed openstack/nova-specs master: Add serial numbers for local disks https://review.openstack.org/556565
15:43:26 dansmith efried: I don't think it really does, this is just the internal way we communicate the request to report client right?
15:43:49 dansmith efried: I'm not really sure why this lives in placement.lib I mean
15:43:50 dansmith because later that will be out of tree and we won't use it to communicate with our own reportclient I think
15:43:50 efried dansmith: yeah. I think the sooner we get to the list-of-tuples representation, the better. So option 2
15:43:58 dansmith mkay
15:44:12 efried dansmith: It's just because RequestGroup is used by both nova side and placement side.
15:44:26 efried on the placement side, we parse the incoming querystring into the exact same representation.
15:44:36 dansmith yeah, this seems like code sharing we should be removing so that we don't have any ties
15:44:41 efried It's like... having a serializable object without having a serializable object.
15:45:23 efried dansmith: Well, cdent is aware, and was involved in the review process (I think). I imagine there will come a time when there will be a placement_lib module that both of them will import.
15:45:58 dansmith I don't see why we'd use that to communicate between internal components of nova, unless it provides a lot of pre-calculation of things or something, which it does not do now,
15:46:22 dansmith but just be advised how hard it will be to land changes to that across both projects and update requirements and such before you can use a new thing if we go that route
15:46:34 cdent I think I expressed reservations at the time, but mostly shrugged in a "we'll figure it out" and "if it helps now, cool" kind of way.
15:46:50 dansmith this provides zero help in its current form, IMHO :)
15:47:13 efried dansmith: Well, only because we haven't closed the final switches on granular yet.
15:47:30 cdent It helps on the placement side to decode the query string, but on the nova side, dunno. I'm not paying huge amounts on the nova side as it is just too hard to keep track of _all_ things
15:47:49 cdent yeah, I think it is groundwork for stuff that was expected sooner than turned out
15:48:11 efried dansmith, cdent: On the nova side it lets us parse extra_specs; on the placement side it lets us parse the querystring. On both sides they parse into the same representation.
15:48:19 efried which is (or will be) useful).
15:48:22 efried ))
15:48:24 efried (((
15:48:25 dansmith cdent: it's just a class with a few variables righ tnow
15:48:41 dansmith anyway, I'm just pre-complaining, nothing that is going to block me right now
15:49:27 cdent dansmith: yeah, I know. I'm mostly speaking generally: I've de-prioritized my attention to the nova side of things for sake of being able to get anything done
15:55:10 openstackgerrit melissaml proposed openstack/nova master: fix a typo in service.py https://review.openstack.org/556575
15:56:00 openstackgerrit Merged openstack/nova master: Updated from global requirements https://review.openstack.org/556418
15:57:09 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM: Add proc_units_factor conf option https://review.openstack.org/554688
15:57:59 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM: Add proc_units_factor conf option https://review.openstack.org/554688
16:06:24 mriedem like killing mosquitoes https://review.openstack.org/#/c/556575/
16:06:40 mriedem https://review.openstack.org/#/c/545528/
16:07:17 mriedem https://review.openstack.org/#/c/555404/
16:07:18 mriedem geez
16:07:20 mriedem yay!
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

Earlier   Later