Earlier  
Posted Nick Remark
#openstack-nova - 2018-01-24
16:41:25 cdent which uses a placement client
16:41:26 edleafe cdent: so IOW, normal for you :-P
16:41:33 cdent edleafe: yes
16:41:59 cdent if the day of the week ending in 'y': chris sick
16:42:09 efried p'tayta p'tata, but okay.
16:42:38 mriedem jackie-truong: L102 https://etherpad.openstack.org/p/nova-ptg-rocky
16:42:43 efried Any placement client wishing to use a cache is broken
16:43:02 cdent I disagree
16:43:11 cdent Any placement client wishing to use a unified cache is broken
16:43:39 cdent I dispute the need for a cache
16:43:44 mriedem gibi: have time to go over this again before your eod? https://review.openstack.org/#/c/536083/
16:44:03 jackie-truong mriedem: Awesome. Thanks for adding that
16:44:04 cdent efried: but it is the road we went down
16:44:09 cdent and I think we can make it work
16:44:31 purplerbot http://p.anticdent.org/4liQ
16:44:31 cdent the things edleafe says at [l 4liQ] seem sane
16:45:00 efried cdent That part has never been under dispute.
16:45:34 edleafe cdent: we seem to have an underwhelming amount of docs about using the generation
16:45:53 cdent I don't doubt that
16:45:57 cdent efried: which part is "that part"?
16:46:28 efried Where we should get a 409 if we PUT something with a generation that's not the latest.
16:47:01 cdent the problems that seem possible from edleafe's flow is that appears to assume a granularity of interaction that it's not clear if the provider tree enables?
16:47:53 edleafe cdent: re: generation doc amount: https://www.youtube.com/watch?v=Ydpablsm7f0
16:48:38 efried cdent I didn't follow that.
16:50:27 cdent What is it that we want/need the generation docs to say?
16:51:24 efried cdent Certainly they need to say, "You need to include whatever generation came in your GET when you do your PUT. If something else updated the provider in between your GET and your PUT, you'll get a 409."
16:51:39 efried cdent One part of the doc says sort of the second half of that, for one API.
16:51:40 edleafe Basically that it is a monotonically-increasing integer, and is increased with each modification of an RP
16:51:55 cdent do we _want_ people to know it is monotonic?
16:51:57 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: Usage docs and initial release note for osc-placement https://review.openstack.org/536858
16:51:57 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: Address review comments from allocations patch https://review.openstack.org/536871
16:52:06 cdent that point seems to not have agreeement
16:52:08 mriedem stephenfin: ok updated the docs one more time to fix a link and to include the resource class stuff which is approved ^
16:52:27 efried cdent Beyond that, they *either* need to say, "We start at zero and increment by 1 whenever something changes about the provider," *or*, "You can't count on how we deal with the generation; it's an opaque value that you need to turn around and send back to us."
16:52:44 efried cdent Yes, I agree that point does not have agreement.
16:53:00 efried If we document the former, then the code as it sits is (almost) okay.
16:53:03 cdent efried why would we want to say the first point above? I think of it as the second.
16:53:10 efried cdent I'm with you.
16:53:18 efried edleafe is on the other side.
16:53:27 efried We don't know what jaypipes thinks
16:53:36 cdent I think edleafe is being pragmatic, more than anything? edleafe ?
16:53:40 efried And I *think* dansmith is also on the "opaque" side.
16:53:43 edleafe if our code makes that assumption, we should document that
16:54:16 efried whoah, we should document something about the API because of something our client code assumes? That's cray-cray.
16:54:20 dansmith efried: yes
16:55:09 dansmith efried: in fact, we could use some random sequence of numbers if we had one and that should be fine. or a uuid. "here is my proof that I'm making a change to the most recent copy" is all we should assume from the client side about the meaning of the generation field, IMHO
16:55:11 efried I'd be willing to bet the client assumptions were made out of expediency to get function merged. At the time it didn't matter because a) single point of control, b) no nested, c) no traits/aggs to complicate.
16:55:18 edleafe efried: the code uses it because it was understood to be the case. If it is going to be the case, it should be documented. If it isn't, the code that assumes that should change
16:55:20 efried dansmith ++
16:55:30 efried edleafe ++
16:55:53 cdent efried: yes, expediency is what I said above
16:56:14 cdent generation are ints because jay likes ints in databases
16:56:18 edleafe We went with a monotonic integer scheme, even though there were better options available. I don't recall why it won out
16:56:25 cdent but that they are ints is incidental
16:56:43 efried So we need to make a call. Course, Jay should be included in the "vote" (if it's a democracy in the first place).
16:57:26 cdent Is the call as narrow as "should we += 1 in the client" or something more broad than that?
16:57:26 efried "opaque" will require some nontrivial redesign and rework
16:58:44 efried cdent The call is, do we document +1 behavior or do we document opacity. What flows from the former is fairly narrow. What flows from the latter is broad.
16:59:38 efried And not to distract, but we still definitely have a problem with DELETE not sending a generation down, no matter which way we decide to go.
17:00:13 dansmith efried: I'm not sure we fully have a problem with delete
17:00:31 dansmith efried: I should be able to delete allocations without a generation without being in violation of anything
17:00:39 dansmith because I know the thing I'm deleting for is never coming back
17:00:58 dansmith what else do we delete for? inventory maybe, but I bet the same argument works for that
17:01:51 cdent efried: I think we can continue to fake on +1 in the ProviderTree if that's what we need to do, and _not_ document it. Other (to be developed) clients won't need to care and should respond to documentation that says what it already says "A consistent view marker"
17:01:52 efried dansmith Inventory, yeah, and agree it's the same argument.
17:02:16 dansmith efried: same argument, but you think we _do_ need a generation on delete right?
17:02:40 efried dansmith Considering the multiple-points-of-control thing, if I think I'm ready to delete the RP's inventory, but you think you're happily editing it...
17:03:01 dansmith efried: but inventory is and should only be managed by the thing owning the inventory
17:03:11 efried dansmith If that's the case, we don't need generations at all.
17:03:28 dansmith efried: I guess the only case that we might hit would be:
17:03:38 efried dansmith I think the problem comes in when deleting the whole inventory is actually a result of "I want to remove inventory in this resource class, but that happens to be the last resource class".
17:03:41 dansmith efried: I'm going to _allocate_ against a RP, right when the RP is deleting the inventory
17:04:19 efried dansmith I think yours is okay. The DELETE will bounce if the allocations happened first; and the allocations will bounce if the DELETE happened first.
17:05:28 dhellmann sdague : are you interested in this testing patch for the nova doc redirects? https://review.openstack.org/#/c/516385/
17:05:48 efried dansmith But for mine: let's say for whatever reason two threads own inventory in different resource classes on the same RP. Thread A decides there's no longer any CUSTOM_FOO, so he goes to remove all the CUSTOM_FOO. He does a GET and sees that the inventory comprises *only* CUSTOM_FOO, so he gets ready to do a DELETE.
17:06:14 cdent efried: I thought the point of the provider tree itself was to lock over that kind of operation?
17:06:26 efried dansmith Meanwhile, thread B decides it's time to add some CUSTOM_BAR. He does his GET, sees some CUSTOM_FOO in there, adds his CUSTOM_BAR, and PUTs the whole shebang back. Now on the server, there's CUSTOM_FOO and CUSTOM_BAR.
17:06:26 cdent the tree can only be manipulated by one thread at a time
17:06:40 dansmith efried: but nothing other than the thing that owns the inventory should collapse a PUT to a DELETE because it thinks it's the last thing, right?
17:07:02 dansmith efried: and the PUT in your case would have included a generation which is wrong now because the DELETE bumped it, no?
17:07:22 efried No, because the PUT happens first.
17:07:27 efried That's the point.
17:07:44 dansmith no I meant the second case
17:08:25 dansmith in the first case, the non-owner is collapsing a PUT to a DELETE which isn't legit It hink
17:08:26 purplerbot <cdent> the tree can only be manipulated by one thread at a time [2018-01-24 17:06:26.123234] [n 1zzs]
17:08:26 cdent efried: is [t 1zzs] not true?
17:08:38 dansmith cdent: it doesn't matter,
17:08:44 efried dansmith I don't disagree. But that's what the code is doing today.
17:08:46 dansmith there could be multiple trees on multiple hosts I think is his point
17:09:02 dansmith efried: I'm saying that's probably wrong, for the non-owner
17:09:06 cdent dansmith: I though we were concluding that the compute node owns the inventory
17:09:08 efried dansmith https://github.com/openstack/nova/blob/b214dfc41928d9e05199263301f8e5b23555c170/nova/scheduler/client/report.py#L982-L985
17:09:14 cdent but I guess some things might share nested bits
17:09:16 cdent which is
17:09:17 cdent awkward
17:10:13 dansmith cdent: we do, I'm just saying I think efried is talking about a more general case of multiple external actors needing to interact with one set of inventory or allocations, using generations as the locking mechanism
17:10:21 efried sean-k-mooney has been coming up with all sorts of interesting use cases where it may very well be the case that e.g. nova owns the VFs, neutron owns the VIFs, and some other thing owns the bandwidth - all on the same RP.
17:10:31 dansmith I'm only half paying attention to this conversation so maybe I should keep quiet :)
17:11:00 efried dansmith So far you've only been reaffirming my position, so by all means continue half paying attention.
17:11:00 dansmith efried: I'm not sure I agree that such a case is legit

Earlier   Later