Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-05
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)
15:58:14 efried cdent: That's precisely my point. It should *not* be placement's responsibility. But therefore it *needs* to be the caller's.
15:58:32 efried cdent: But with the API as it stands, we provide the caller no mechanism to enable it to execute that responsibility.
15:58:40 dansmith mriedem: we kinda disabled the on_shared_storage flag, IIRC.. are we good to remove that in the rpc bump or do we need to keep it so the api can ask us to check it for compat?
15:58:46 efried cdent: Generation would be one such mechanism.
15:59:11 edleafe efried: so would something like Watcher, which can re-balance things at the cloud level
15:59:26 cdent efried: the caller can do what it likes, managing data how it likes, but it shouldn't rely on placement to do it for it, at the level of allocations. It could make requests for allocation candidates how it likes, look at them, analyze then, do whatever.
15:59:28 mriedem dansmith: are you asking b/c of gibi's functional test?
15:59:31 mriedem or b/c you're bumping major versions?
15:59:35 efried edleafe: After the allocation has been made and the instance has been scheduled? Sure. Messy, though.
15:59:37 edleafe efried: IOW, detect a crowded host and performt the migration to uncrowd it
15:59:53 edleafe efried: it
15:59:57 edleafe it's a cloud
16:01:08 efried edleafe: Right, I get it. I'm saying migration-after-the-fact is a viable solution, but it's more expensive/disruptive to the workload than getting the scheduling "right" in the first place.
16:01:38 edleafe efried: it *will* get it right the overwhelming majority of the time
16:01:51 edleafe The case we're describing is an edge of an edge case
16:02:07 efried edleafe: Actually, I think it'll get it "wrong" (for common definitions of "wrong") quite often.
16:02:27 efried edleafe: Only when lots of concurrent deploys are happening, of course.
16:02:30 dansmith mriedem: bumping major versions
16:02:49 efried edleafe: Or concurrent migrations (such as an "evacuate host" scenario).
16:03:24 gibi mriedem: I'm in the process to updat that patch not to depened on that flag
16:03:30 cdent efried: if you're a person deploying 3 instances and the mechanism you want to use to ensure they go different places is "not loaded please", that sounds wrong. Presumably what you really want is anti-affinity and not loaded.
16:03:50 edleafe efried: https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L411-L422
16:03:50 cdent If you're just going with "not loaded" and you get something weird, isn't that you're own fault?
16:04:13 cdent sigh: your
16:04:18 efried cdent: I mean, we can document it that way, but it's not very satisfying to the consumer.
16:04:30 efried cdent: And I don't agree that you necessarily want anti-affinity.
16:05:02 edleafe efried: and also https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L426-L436
16:05:02 efried If host A is 10% full and host B is 90% full, I totally want my instances (each of which consumes 10%) to both go to host A.
16:05:24 edleafe There is plenty of race avoidance in the code.
16:05:49 cdent I dunno, I think your host should just go where it will get what it needs, unless you tell it something special.
16:06:03 cdent And providing too many knobs is bad for everyone
16:07:08 efried edleafe: Okay, glad those mechanisms are there, but they're somewhat inelegant. (Not that thrashing GET /allocation_candidates is any more elegant - overall I think it's clunkier and less efficient - but as an example, 'twill serve.)
16:07:23 mriedem dansmith: i'd have to take a look at that flag to see what the api does with it
16:07:40 efried cdent: Too many knobs, sure, but I thought a "least consumed" weigher was one of the basics.
16:07:43 dansmith mriedem: okay I thought you did it which is why I was asking.. I'll add a note and come back to it
16:07:56 openstackgerrit Merged openstack/nova stable/pike: doc: Add user index page https://review.openstack.org/540494
16:08:02 openstackgerrit Merged openstack/nova stable/pike: Migrate "launch instance" user guide docs https://review.openstack.org/540495
16:09:06 cdent efried: is it? to me it sounds like a "exposing too much info to the user about this cloud" and "our resource accounting and allocation ratio subscription model is actually a cheat, here we've provided you with this way to get around it rather than being good cloud providers with good ratios"
16:09:13 mriedem dansmith: no i don't think so, sounds like a tdurakov thing
16:09:36 dansmith mriedem: ack
16:10:40 efried cdent: I don't see what this has to do with allocation ratios.
16:11:16 cdent if allocation ratios are set fairly and correctly, then where your instance goes shouldn't matter: it should always have good access to resources whatever host it lands one
16:11:43 cdent if hosts are oversiubscribed then of course you're going to try to do tricks to get on an underutilized host
16:12:39 efried cdent: That makes a lot of assumptions about the possible reasons for a "least consumed" deployment strategy.
16:13:05 cdent what other reason can you want other than needing to ensure you "get enough"?
16:13:53 efried cdent, edleafe: Anyway, y'all have convinced me we don't need/want this for Nova. So until/unless we get a request from some other placement consumer (which isn't quite so likely to happen as long as placement is part of nova) I can drop it.
16:18:04 openstackgerrit Balazs Gibizer proposed openstack/nova master: Add regression test for bug 1735407 https://review.openstack.org/526095
16:18:06 openstack bug 1735407 in OpenStack Compute (nova) "[Nova] Evacuation doesn't respect anti-affinity rules" [Medium,In progress] https://launchpad.net/bugs/1735407 - Assigned to Balazs Gibizer (balazs-gibizer)
16:18:56 gibi mriedem: ^^ fixed the functional test, now start fixing the bugfix patch
16:26:15 mriedem dansmith: looks like gibi made onSharedStorage optional in evacuate
16:26:36 dansmith mriedem: right, but it was required in earlier api versions
16:26:37 openstackgerrit Jay Pipes proposed openstack/nova master: only increment disk address unit for scsi devices https://review.openstack.org/538310
16:26:51 mriedem dansmith: yeah, so it looks like we could kill it from compute...
16:26:52 dansmith so I'm wondering if we have to keep the yes/no/unspecified support in the lower layers
16:27:19 jaypipes melwitt, mdbooth: really need some assistance from either/both of you on https://review.openstack.org/538310. Please see my desperate cry for help in the second unit test there...
16:27:20 mriedem unless there is some scenario where operators actually need to rely on it b/c the auto-check using the driver is wrong,
16:27:23 mriedem but that would seem like a bug
16:27:49 dansmith mriedem: well, the point being the api used to advertise that it will check your work
16:27:59 dansmith so if people used that, and we silently ignore it in compute now...
16:28:22 mriedem it used it for some other stuff too, related to the adminPass
16:29:23 mriedem reading the spec https://specs.openstack.org/openstack/nova-specs/specs/mitaka/implemented/optional-shared-storage-flag-in-evacuate-api.html

Earlier   Later