Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-05
14:54:15 gibi yes, we need to agree on such a bump together. Especially as dansmith might plan to do a major rpc version bump before the release
14:54:46 dansmith yeah, unfortunately, that reqspec cleanup would be really nice to get into place
14:56:30 mriedem i got 2 patches in against it for queens!
14:56:31 gibi bauzas: if that helps with the current bugfix, I can offer some help with the request spec blueprint in Rocky
14:56:33 mriedem with great success
14:57:29 efried bauzas: What's a request spec, as distinct from allocation request data?
14:58:01 efried That is, what does a request spec has that an allocation request doesn't?
14:58:19 mriedem gibi: replied in https://review.openstack.org/#/c/528385/
14:58:32 mriedem efried: all sorts of stuff,
14:58:38 mriedem instance groups, scheduler hints,
14:58:41 mriedem image id
14:58:46 mriedem it's basically the instance
14:58:53 mriedem minus host stuff (and BDMs for some reason)
14:59:03 efried okay, thanks.
15:00:20 efried cause jfyi, we are passing the allocation to the compute (https://review.openstack.org/#/c/511879/)
15:00:32 bauzas efried: originally the request spec is what was passed to the scheduler for filtering purposes
15:00:44 mriedem efried: in this case we need the scheduling hints
15:00:52 efried coo
15:00:58 bauzas efried: it became used for more than just the scheduler
15:01:20 bauzas because we have the original intent of the user + some quirks
15:01:57 bauzas the flaw in that model (and I apologize for not having considered it at the design state) is that we don't know which fields are immutable and which not
15:02:25 gibi mriedem: thanks the reply in https://review.openstack.org/#/c/528385/, I'm OK with the extra kwargs
15:02:36 bauzas so, when you review a change using that RequestSpec object record, try to consider all fields as potentially mutable
15:03:10 gibi ameeda: I think you have a way forward now in https://review.openstack.org/#/c/528385/
15:04:08 openstackgerrit Peter Hamilton proposed openstack/nova-specs master: Add support for certificate validation https://review.openstack.org/540879
15:16:54 lyarwood mriedem: https://review.openstack.org/#/c/531422/ - happy to +W this now that https://review.openstack.org/#/c/540680/1 is posted?
15:25:09 mriedem gibi: comments in https://review.openstack.org/#/c/525242/
15:26:51 gibi mriedem: looking
15:28:10 cdent efried: sorry, was walking. On the POST, do the nearby links clarify things at all? On the allocations have a resource provider with a generation that's here: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/allocation.py#L210-L220
15:29:01 openstackgerrit Stephen Finucane proposed openstack/nova master: doc: Start using openstackdoctheme's extlink extension https://review.openstack.org/540888
15:29:01 openstackgerrit Stephen Finucane proposed openstack/nova master: doc: Don't use single backticks in man pages https://review.openstack.org/540887
15:33:06 mriedem gibi: and the functional test below
15:33:44 mriedem dansmith: melwitt: easy quick cells v2 layout caveats update for stuff that is now fixed in queens https://review.openstack.org/#/c/540667/
15:34:01 gibi mriedem: thanks
15:34:26 mriedem bauzas: this looks nice https://docs.openstack.org/nova/latest/admin/virtual-gpu.html thanks for doing that
15:40:10 efried cdent: I knew what you meant; was just commenting that the sentence didn't grammar for me.
15:40:38 cdent efried: because of the missing "of" or more than that?
15:41:20 efried cdent: TBH, I couldn't even interpolate what was missing/wrong. Let me go read it again...
15:41:28 cdent i've just put the "of" in
15:41:38 mriedem lyarwood: done
15:42:00 lyarwood mriedem: thanks
15:42:20 efried cdent: Oh, yeah, that (or the addition of caffeine since first reading) makes it a lot clearer.
15:42:37 efried cdent: And when you get a chance, you said you could point me to where allocations were managing generations.
15:42:41 mriedem lyarwood: can you hit the other 2 pike doc backports here? https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/pike+topic:bug/1714017
15:43:03 lyarwood mriedem: sure, looking now
15:43:09 purplerbot efried: sorry, was walking. On the POST, do the nearby links clarify things at all? On the allocations have a resource provider with a generation that's here: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/allocation.py#L210-L220 [2018-02-05 15:28:10.592679] [n vPW]
15:43:09 cdent efried: second part of [t vPW]
15:43:51 efried cdent: That's on the return from POST /allocations?
15:44:02 mriedem lyarwood: i'm not sure about this https://review.openstack.org/#/c/540846/
15:44:19 mriedem it's going to be super annoying if we have to update all doc backports to point at the branch they are on
15:44:19 cdent that gives the Allocation object and rp (with a genaration), which is then checked at https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L2079-L2085
15:44:25 mriedem we don't do that when we branch
15:44:29 cdent efried: that's on the _start_ of PUT or POST
15:44:35 cdent on the way in, not the way out
15:44:41 openstackgerrit Merged openstack/nova master: Fix invalid UUIDs in test_compute.py https://review.openstack.org/531479
15:44:49 cdent efried: so the generation is checked, but the client doesn't provide
15:44:50 openstackgerrit Merged openstack/nova stable/pike: Don't update RT in _allocate_network https://review.openstack.org/519942
15:44:55 openstackgerrit Merged openstack/python-novaclient master: Zuul: Remove project name https://review.openstack.org/540304
15:44:56 cdent which seems ... useless?
15:45:04 efried cdent: Right. What are we checking the generation *against*?
15:45:30 efried cdent: (BTW, in that code block, I still don't see where the generation is being checked, even internally)
15:45:43 cdent efried: anything that changes from the start of the PUT to the time when _increment_provider_generation is called
15:46:13 bauzas mriedem: thanks for pushing me the idea :-)
15:46:22 efried cdent: Okay, well, I should hope that *all* the APIs are doing *that*, to guarantee atomicity of a single call. But yeah, totally not the point I'm actually making in the bug report.
15:46:43 cdent efried: yes, your bug report is a different concern
15:46:54 cdent and no, we shouldn't be doing that in all single calls
15:46:58 cdent it is pointless in allocations
15:47:00 mriedem lyarwood: are you planning on backporting this entire series to ocata also https://review.openstack.org/#/c/540861/ ?
15:47:03 cdent if the allocation fits, write it
15:47:04 mriedem that's a lot of refactor
15:47:11 cdent efried: but we don't currently do that
15:47:34 lyarwood mriedem: still working out if that's even possible
15:47:46 efried cdent: So that's the point we need to consider. I'm not sure I agree that we should always write the allocation if it still fits.
15:47:51 lyarwood mriedem: yeah, the refactor also fixes a load of connect/disonnect issues
15:48:22 lyarwood mriedem: really I should've landed a smaller bugfix ahead of this on master to backport
15:49:03 mriedem hmmm,
15:49:04 mriedem like what?
15:49:05 cdent efried: if you can come up with a cogent explanation of why, that would be great, but to me it violates the spirit of allocations to do anything else and also over constrains the system.
15:49:22 mriedem lyarwood: why not just put a change on stable that says if you're trying to swap encrypted volumes, we fail?
15:49:39 efried cdent: Okay, example: since we're explicitly *not* handling weighing in placement, supporting generation would be a way the caller (who has to handle weighing) can ensure the weighing assumptions they make in the PUT/POST allocation call actually apply at the time when the allocation is written.
15:50:13 lyarwood mriedem: yup that works and avoids this horrid backport, I'll do that now.
15:50:14 mriedem lyarwood: since swap volume doesn't work with encrypted volumes until queens (and only if you're not using native luks encryption), it seems ok to just make it outright fail on stable
15:50:54 cdent efried: "can actually apply" shouldn't mean generation (when it comes to allocations) it should mean capacity/capability
15:51:46 cdent (or if I'm not understanding you, you'll need to be more complete with your example)
15:51:53 efried cdent: I agree with that, but "should actually apply" is based on assumptions about the state of providers when the request was calculated.
15:52:32 cdent which is why we 409 if that state has changed, in a way that changes capacity, by checking the capacity at allocation write-time
15:52:49 efried cdent: Yeah, capacity conflicts I'm cool with, put that aside.
15:52:55 edleafe cdent: I think jaypipes expressed something similar a while ago. If a deployment is trying for spread, and two requests to allocate against the same host come in, the second should fail
15:53:03 cdent if we want to rely on generation, then we are vastly decreasing the odds of an allocation being able to land
15:53:16 edleafe IMO, that's a huge nova-ism for placement to base its behavior on
15:53:32 efried Yes, we may want to consider making the generation handling optional for allocations. But it should at least be possible.
15:53:45 cdent efried: I still haven't heard why?
15:54:04 cdent was " is trying for spread, and two requests to allocate against the same host come in, the second should fail" a reason why? If so, I don't understand?
15:55:19 efried cdent: Yeah. Scheduler is scheduling three instances. Each of the three threads requests allocation candidates "at the same time". Gets back the same list. Scheduler applies a "least consumed" weigher in each of those three threads, and winds up sorting the candidates in the same order. Scheduler sends all three PUT /allocations/{c} at the same time, with the same allocation request in each.
15:55:28 edleafe cdent: because nova thinks the host is relatively empty, so it chooses it. But had the first allocation been known, it would have chosen an emptier host for the second
15:56:31 efried cdent: Even assuming there's capacity for all three on the same set of RPs, by the time we've allocated the first one, the "least consumed" condition, upon which the second scheduler thread's PUT request was based, no longer applies.
15:57:04 efried cdent: Which may be an acceptable limitation of the scheduler's weigher. But it may not be.
15:57:21 efried cdent: Even more so if we have shared RPs involved, and these threads could be hitting from multiple separate compute nodes.
15:57:33 edleafe efried: this all seems like a good argument for placement not being part of nova
15:57:45 cdent a) that's not a thing I think placement should concern itself with, b) that's not a concern I think a cloud should concern itself with (in that fashion, it shoul be orchestrated at a different layer)

Earlier   Later