Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-07
17:01:02 stephenfin Damn - I skimmed down to NovaTimestampObject and went no further
17:01:11 stephenfin Fairs. That's done now
17:02:21 mriedem thanks
17:07:25 stephenfin mriedem: Side note - do we have a patch to populate this? Looking for review stuff to avoid more bug fixing https://specs.openstack.org/openstack/nova-specs/priorities/queens-priorities.html
17:09:35 efried mriedem The plan is to have ComputeDriver.update_provider_tree, which we pass in a ProviderTree object. The driver will likely have to know its own compute node UUID so it can find it in that tree. But when it's done, the report client will take the updated ProviderTree and flush changes back to placement.
17:09:49 efried bauzas ^
17:10:24 efried mriedem bauzas There's some rough WIP at the top of the NRP series that gets a start on that. I'm noodling with it some more now.
17:10:36 mriedem stephenfin: i haven't done it yet
17:11:42 bauzas efried: ok, gtk
17:11:55 bauzas efried: so the driver will know the tree, all good with me
17:12:43 efried bauzas Well, yes, the driver will mostly be responsible for *creating* the tree. What it receives initially will probably just be the compute node provider (and maybe its traits).
17:12:56 bauzas k
17:13:21 efried bauzas At that point it'll be responsible for creating nested RPs (e.g. numa nodes, SR-IOV PFs, etc.) and possibly even sharing RPs; and setting up traits & aggregate associations.
17:14:32 bauzas yeah I remember now that convo
17:18:53 efried jaypipes cdent Here's a wrinkle: Can sharing RPs be trees?
17:19:02 efried Hint: it'd be a lot cooler if they couldn't.
17:19:20 efried I mean, it's clear that nothing is preventing you from doing that today.
17:19:27 efried I'm saying we should consider whether that's a good idea.
17:23:33 mriedem cdent: do you think https://review.openstack.org/#/c/518223/ is something we need to backport to pike?
17:24:16 cdent mriedem: is a nice to have but not a need to have
17:25:27 cdent efried: I think you and I have different mental models of rp trees. I think it should be theoretically fine for an rp which _happens_ to be part of a tree to be shareable but that it is in a tree shouldn’t be particularly meaningful
17:25:48 efried cdent Okay, yeah, I think I'm discovering that.
17:26:16 efried cdent This is coming from me trying to make sure the ProviderTree is fully populated before we hand it off to the compute driver.
17:26:25 cdent what tree-ness would mean in that context is a path to get to
17:26:59 efried I have to spider out from the compute RP's tree, grab all the RPs associated via aggregate, and populate the ProviderTree with those guys.
17:27:16 mriedem cdent: ok
17:27:23 efried So I was wondering what happens if one of those sharing RPs happens to be a child - do I *also* have to grab his whole tree and populate the ProviderTree with *those* guys.
17:27:39 efried The answer I'm coming to is: No, I just pretend he's a root.
17:27:45 cdent yes
17:28:12 cdent at least at a first pass. it feels like you are working a bit ahead of yourself. solve the problem you have in front of your face, not all the problems you can imagine?
17:28:13 efried So I may end up with multiple agg-associated sharing RPs that happen to be in the same tree, but they'll appear flattened as roots in the ProviderTree.
17:28:40 efried cdent I agree with that in principle, but I have to code *something* here.
17:29:22 cdent efried: I assume you saw my comments on your commit messages needing to say more about why you are coding these *somethings*?
17:30:14 efried cdent One of them, which I updated - see if it satisfies: https://review.openstack.org/#/c/521097/
17:31:36 cdent efried: is there a spec or something where the reasons for the reason you gave (ComputeDriver.update_provider_tree) are explained?
17:31:55 efried jaypipes ^?
17:32:24 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: [placement] Fix an error message in API validation https://review.openstack.org/526465
17:33:03 cdent efried: I mean, I’m willing to take it as good simply because you guys are committed to it, but it easier to revie when I have a bigger picture
17:33:28 efried cdent I agree it would be neat to have a spec that calls out ComputeDriver.update_provider_tree.
17:34:00 efried cdent Does the WIP patch that introduces that method help at all? https://review.openstack.org/#/c/521187/8/nova/virt/driver.py
17:34:46 efried not much, I guess.
17:36:08 cdent The “supersedes get_inventory()” is probably the most meaningful bit
17:36:57 cdent and interesting in the sense that for most of the virt drivers we’ll introduce and then supersede get_inventory in the same cycle (if lucky, subsequent otherwise)
17:37:43 efried cdent Yeah, I think PowerVM will get to skip get_inventory()
17:37:55 efried but it'll be the exception.
17:38:04 efried Meh, actually I don't know.
17:38:33 efried We should have update_provider_tree ready to go this cycle; any virt drivers that don't already have get_inventory() wouldn't *need* to implement it. Though I suppose they may choose to.
17:39:51 jaypipes efried: haven't thought about that, frankly
17:40:27 efried jaypipes I think cdent and I talked ourselves into relative sanity on the subject.
17:41:53 mriedem melwitt: https://review.openstack.org/#/c/526182/ and the one above it have +2s from gibi
17:41:58 mriedem for the new volume attach stuff
17:53:54 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Fix NoneType error when [service_user] is misconfigured https://review.openstack.org/526468
18:43:05 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Enable limiting GET /allocation_candidates https://review.openstack.org/513526
18:43:15 cdent one down, one to go
18:43:43 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Only log not correcting allocation once per period https://review.openstack.org/526477
18:58:52 efried jaypipes cdent edleafe dansmith I'm about to go through SchedulerReportClient and make all the methods raise execptions when appropriate instead of returning None. Would you prefer I did that all in one patch, or split it into many (e.g. one per method or group of related methods)?
18:59:37 dansmith is there something we need to be catching that we can't without raising there?
18:59:45 cdent my preference would be for many: makes any booms a bit more obvious
18:59:54 dansmith but, I'm sure multiple patches per call would be way easier
18:59:58 dansmith also what cdent said
19:01:56 edleafe efried: yeah, smaller chunks are better
19:02:34 efried dansmith Well, we caught a grenade bug (it went "boom") with the last one.
19:03:35 efried dansmith It's not that we *couldn't* catch the errors without raising; it's that we *don't*. We're not paying attention to the returns from these methods in many cases, so the errors slide on by without anyone noticing.
19:04:13 dansmith well, in some cases that's intentional, where we get allocations and if none, then do a thing
19:04:26 dansmith just saying all those places will need to be changed. I'll reserve judgment until I see them I guess
19:06:18 efried dansmith Roger wilco. I think the important ones are where the caller doesn't necessarily have to do anything with the return value from the method - just invoking for the side effect. Methods like get_allocation_candidates don't fit that, so I guess it makes sense to leave 'em alone.
19:09:48 dansmith I just hadn't thought much about it so seeing them all will help
19:11:18 efried dansmith With that discussion, it's actually going to be pretty limited. So thanks for that.
19:29:31 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Object changes to support last-modified headers https://review.openstack.org/521639
19:29:32 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Add cache headers to placement api requests https://review.openstack.org/521640
19:29:32 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Add info about last-modified to contrib docs https://review.openstack.org/526084
19:38:55 jaypipes dansmith, mriedem: https://review.openstack.org/#/c/385693/ should be ready to go.
19:39:43 mriedem it's placement
19:39:45 mriedem i'll tell you now
19:40:06 jaypipes :(
19:40:10 mriedem jaypipes: i'll trade you that for https://review.openstack.org/#/c/525787/
19:40:42 dansmith I dunno what jaypipes' is, but it's not an even trade
19:40:52 jaypipes mriedem: sure, deal.
19:41:45 mriedem dansmith: is that a slight against mine or his?
19:41:56 dansmith mriedem: yours :)
19:42:01 mriedem ha
19:42:11 mriedem everyone loves some good block device'n
19:42:14 efried dansmith Yeah, don't take the trade. jaypipes' is easy.
19:42:44 dansmith I clicked on mriedem's link to see if it was one of my reviews he just just finished and was looking for a +W
19:42:49 dansmith but alas, no
19:43:00 jaypipes efried: not *just* BDM. BFV + BDM, otherwise known as BDSM.
19:43:05 dansmith haha
19:43:12 mriedem dansmith: you can get this easy refactor to complete the set https://review.openstack.org/#/c/526182/
19:43:52 dansmith mriedem: I'll trade you for revisiting my live migration allocations one
19:44:12 dansmith that will even out your karma for trading jay something easy for something hard
19:44:33 mriedem fwiw, https://review.openstack.org/#/c/385693/ has a problem in the commit message
19:46:05 efried mriedem Will y'all fast-approve if I make that edit? Hate to lose gibi's +2
19:46:19 mriedem i'm still reviewing
19:46:24 mriedem there are other....concerns
19:46:28 efried ight
19:46:34 mriedem the ... is for intended dramatic effect
19:46:46 efried jaypipes FYI I have this series locally, with lots of deltas, so *please* don't re-publish.
19:46:55 jaypipes efried: roger
19:50:20 mriedem efried: jaypipes: replied https://review.openstack.org/#/c/385693/
19:50:22 mriedem pick your poison

Earlier   Later