| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-24 | |||
| 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 | cdent | the things edleafe says at [l 4liQ] seem sane | |
| 16:44:31 | purplerbot | http://p.anticdent.org/4liQ | |
| 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: Address review comments from allocations patch https://review.openstack.org/536871 | |
| 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: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 | efried | "opaque" will require some nontrivial redesign and rework | |
| 16:57:26 | cdent | Is the call as narrow as "should we += 1 in the client" or something more broad than that? | |
| 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 | cdent | the tree can only be manipulated by one thread at a time | |
| 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: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 | cdent | efried: is [t 1zzs] not true? | |
| 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: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 | dansmith | efried: I'm not sure I agree that such a case is legit | |
| 17:11:00 | efried | dansmith So far you've only been reaffirming my position, so by all means continue half paying attention. | |
| 17:11:03 | cdent | efried: in which case a cache that is local the compute node is going to fraught with challenges beyond the one you have identified today | |
| 17:11:12 | efried | dansmith Once you start disagreeing, then yeah... | |
| 17:11:16 | dansmith | hah | |
| 17:12:07 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Don't filter out sibling sets with one core https://review.openstack.org/537361 | |