Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-02
18:59:30 openstackgerrit Ed Leafe proposed openstack/nova master: Add handlers for consumer_generation https://review.openstack.org/557959
18:59:30 openstackgerrit Ed Leafe proposed openstack/nova master: Add columns for generation to consumers and allocations https://review.openstack.org/557958
19:01:48 fried_bunny jaypipes: did you catch up on the shared provider stuff we were discussing with bhagyashris?
19:02:31 fried_bunny jaypipes: I wanted to get a sense for whether that constitutes a regression we need to fix, or whether it figures into the whole "we don't support shared yet" picture.
19:24:01 openstackgerrit Vladyslav Drok proposed openstack/nova master: Ignore some updates from virt driver https://review.openstack.org/523006
19:32:30 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: Snapshot https://review.openstack.org/543023
19:32:59 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053
19:35:18 openstackgerrit Jay Pipes proposed openstack/nova master: Get rid of 406 paths in report client https://review.openstack.org/556633
19:35:30 jaypipes fried_bunny: was a simple rebase ^
19:36:17 fried_bunny jaypipes: Roger. Why was it needed?
19:36:36 fried_bunny because I forgot to do it after editing the previous in gerrit?
19:36:41 jaypipes fried_bunny: the previous version was based on an old patch.
19:36:47 jaypipes fried_bunny: yep
19:36:57 fried_bunny ight
19:45:42 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053
19:48:33 fried_bunny edleafe: jaypipes: How does a consumer get created? Implicitly when you create allocations for it?
19:49:16 jaypipes fried_bunny: yes
19:49:18 fried_bunny Then what do you send down as your generation value the first time you create such an allocation? I guess None has to be legal at that point.
19:49:54 jaypipes fried_bunny: yeah
19:50:10 fried_bunny edleafe: ^ how is this handled?
19:50:54 fried_bunny Under the new microversion, None has to be special
19:51:10 fried_bunny special-cased to mean "Fail 409 if the consumer exists"
19:52:09 jaypipes fried_bunny: the previous behaviour is essentially an implicit consumer_generation of None.
19:52:20 fried_bunny jaypipes: Not the same thing at all, though.
19:52:28 jaypipes fried_bunny: how so?
19:53:29 fried_bunny With the new code in place, you can still use the old microversion, which is consumer_generation of None, and it'll increment the existing generation, whatever it happens to be.
19:53:39 fried_bunny If you use the *new* microversion and send None, we can't do that.
19:54:51 jaypipes fried_bunny: why would you send the new microversion and send None, though?>
19:55:07 edleafe fried_bunny: that's how it's supposed to work
19:55:16 fried_bunny jaypipes: Because you *have* to do that when the consumer doesn't exist yet.
19:56:04 fried_bunny Cause we had the discussion about opacity - made sure *that* was in the spec. So you can't send 0. But if you do a GET, you'll get 404 cause the consumer doesn't exist yet.
19:56:12 fried_bunny so you don't have an existing generation to populate your payload with.
19:56:18 edleafe fried_bunny: you will have gotten the generation from GETting existing allocations
19:56:20 openstackgerrit Dan Smith proposed openstack/nova master: Fix cancel_all_events event name parsing https://review.openstack.org/558059
19:56:28 fried_bunny edleafe: Not when there ain't none yet.
19:56:28 edleafe if there is no consumer, you would get None
19:56:39 fried_bunny you would get 404
19:56:43 fried_bunny not so?
19:56:50 jaypipes fried_bunny: but the only reason to have this consumer generation at all was for the PUT /allocations/{consumer_uuid} when you wanted to modify an existing allocation.
19:56:58 edleafe you would get an empty list
19:57:10 fried_bunny jaypipes: But how do you know whether you're modifying an existing or creating a new?
19:57:44 jaypipes fried_bunny: when you're trying to modify an existing allocation, it's a totally different call point than when you're creating a new allocation for a new instance.
19:58:20 fried_bunny jaypipes: Maybe for Nova it is.
19:58:20 jaypipes fried_bunny: the scheduler always does claim_resources() which is new consumer (new instance). the conductor will be the only thing modifying existing allocations.
19:58:24 edleafe either way, you're calling set_allocations, no?
19:58:30 fried_bunny ^
19:58:41 jaypipes edleafe: claim_resources() vs. some new modify_allocations() call.
19:59:17 jaypipes the only reason we need consumer generation is when we're modifying existing allocations.
19:59:23 jaypipes there's no need for it anywhere else.
19:59:27 fried_bunny From an API perspective, though, you're calling PUT /allocations/{c}
19:59:35 fried_bunny (or POST, same-same)
19:59:55 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: Snapshot https://review.openstack.org/543023
19:59:55 edleafe ok, I think I see the case fried_bunny is talking about
20:00:00 fried_bunny edleafe: confirmed you get 200 empty list when calling on a consumer that doesn't exist.
20:00:21 jaypipes fried_bunny: then have claim_resources() use microversion 2.11 and have modify_allocations() use microversion 2.42 (or whatever microversion this new change gets)
20:00:39 edleafe it would happen if you created an instance, and then somehow un-allocated all its resources, and then re-allocated them.
20:00:50 edleafe I don't think that's possible
20:01:02 fried_bunny or a race where two agents attempt to create the first alloc for a consumer ID.
20:01:17 fried_bunny Sure, we can say it'll never happen in nova (maybe).
20:01:35 edleafe fried_bunny: yeah, and the second will fail that race
20:01:38 edleafe as designed
20:01:41 fried_bunny But from a pure API perspective, the point of this exercise is to protect against clients racing to PUT /allocations/{c} - whether it's the first or not.
20:02:13 fried_bunny edleafe: That's what I'm saying. If the consumer doesn't exist yet, we *must* have a special case for consumer_generation=None which fails 409 if the consumer exists at all.
20:02:32 fried_bunny We *can't* have it just increment the generation in that case. Or the *first* guy will lose the race, and not know why.
20:02:48 edleafe fried_bunny: let's walk through the race
20:02:52 fried_bunny And, this is a weird behavior, so it needs the shite documented out of it.
20:02:53 fried_bunny okay.
20:03:03 fried_bunny me or you?
20:03:09 edleafe both agents get the current state for the allocations, which is empty
20:03:12 edleafe me
20:03:31 edleafe Both try to PUT their allocations with con_gen=None
20:03:46 edleafe One succeeds, and the generation is then set at 0
20:03:55 edleafe The second fails with a 409 response.
20:04:22 edleafe The second then refreshes its view, and gets allocations this time, with con_gen=0
20:04:43 edleafe The second adds its allocations to that, and PUTs with a con_gen of 0
20:04:45 fried_bunny yeah, the behavior you describe is what needs to happen.
20:04:52 fried_bunny But I don't think that's the way it's coded up.
20:04:54 fried_bunny could be wrong.
20:04:57 edleafe The second succeeds this time, and now con_gen=1
20:05:15 fried_bunny I think the way it's coded up, you're assuming None means "old microversion, so just increment"
20:05:32 edleafe What I worry about is the second agent posting to an old microversion
20:05:40 fried_bunny That's his fault.
20:05:45 edleafe Which means "preserve the existing behavior"
20:05:50 edleafe which means overwriting
20:06:50 fried_bunny As far as that guy is concerned, the generation field doesn't exist. For the sanity of the first guy, we have to increment it. But if you're trying to use allocations from multiple clients and you're not using the generation-supporting microversion, you're toast anyway.
20:06:58 edleafe It's (very) possible that I missed a case in the code. But the approach is what was designed
20:07:45 edleafe agree that even if there is a post with an old microversion, the generation should be incremented
20:08:36 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053
20:08:52 fried_bunny edleafe: I'm going through the spec again, but I don't recall this case being covered.
20:09:09 fried_bunny It needs to be described explicitly, because it's weird.
20:10:09 fried_bunny "Here's a new microversion where you have to send generation to ensure consistency. Oh, except the first time, when you have to send None (or leave it empty??). But if you do that when the consumer already exists, you'll fail.
20:10:11 fried_bunny "
20:11:46 edleafe it might not be in the spec
20:12:23 edleafe But it's not that weird. Standard practice is to get existing allocations before adding any new ones
20:12:49 edleafe I know it was discussed
20:12:52 fried_bunny okay, so what will you return?
20:12:55 fried_bunny in that first one?
20:13:00 fried_bunny Right now you get {}
20:13:04 edleafe but those discussions happened in several places at various times
20:13:12 fried_bunny Will you return { "consumer_generation": None } ?

Earlier   Later