Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-24
16:05:07 efried And it's not mentioned in the overview doc (https://docs.openstack.org/nova/latest/user/placement.html) at all
16:05:44 edleafe efried: I'll have to do some doc digging, but I'm sure it's there
16:06:04 edleafe efried: here's the flow I see for your inventory delete/update:
16:06:06 edleafe You have a cached RP with generation 5. You call DELETE for the inventory, and it succeeds. You now update your cache with generation 6, and no inventory.
16:06:10 edleafe You now want to PUT some inventory. So you send the PUT with the data and generation 6. Something else has changed the RP in the meantime, so your PUT returns 409. At that point, your cache is not accurate. You don't know what has changed, so you get rid of anything for that RP, and call GET with the rp_uuid to refresh everything in your cache for that RP. You then compare the inventory you
16:06:16 edleafe expected with what is there (which you would have done before the original PUT), and if it is still correct to update inventory, you re-PUT the inventory with the new generation. Rinse and repeat.
16:06:56 efried edleafe That's a good flow, but it's not the one I described.
16:07:13 efried edleafe But before we go on, how does "You now update your cache with generation 6" happen?
16:07:43 edleafe that was the assume that if your DELETE succeeds at gen5, it is now at gen6
16:07:50 efried Assuming gen+1?
16:08:13 melwitt mriedem: ack
16:08:27 edleafe efried: yes.
16:08:54 efried Okay, then I agree it works, but is illegal API consumption. Because the API doesn't tell us we can assuming monotonically increasing generation counter per operation.
16:09:22 edleafe efried: if it doesn't, then that's a doc bug
16:09:49 efried This is how we do it today. Which is (part of) the reason we don't have a "bug" per se. (The other part is that we don't actually have any paths that do concurrent updates yet.)
16:10:01 efried edleafe I don't agree with that.
16:11:40 edleafe efried: let's get jaypipes and cdent to weigh in on this
16:11:49 efried edleafe But maybe. We should find out what Jay thinks. If we add words to those DELETE 204s along the lines of, "You may now assume the resource provider generation has been incremented by 1," I'll stfu.
16:11:56 efried yeah, what you said.
16:11:58 edleafe I think we understand each other, and are not in agreement
16:12:08 efried Does dansmith have skin in this game?
16:12:27 dansmith um
16:12:29 edleafe I think dansmith is pretty busy with other things :)
16:12:37 efried k
16:13:07 edleafe efried: the assumption is that after anything that modifies anything about a RP, the generation is +1 from what it was
16:13:14 edleafe not just DELETE
16:14:19 dansmith client-side ever predicting the generation is wrong
16:14:26 dansmith you would never do gen+=1 locally
16:14:30 efried ++
16:14:37 dansmith you have no idea if your generation 6 is the same as the server side
16:14:46 efried https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L927
16:14:51 dansmith you think so because you posted with 5 and it succeeded, but you don't know
16:15:44 edleafe dansmith: which is why you always need to expect a 409
16:16:00 edleafe otherwise, what's the point of caching anything?
16:16:59 efried Which is actually another good point. DELETE /rp/{u}/inventories doesn't accept a generation (because doesn't accept a payload).
16:17:25 edleafe efried: ah, now *that's* a problem
16:19:21 efried Here's the other place we're making assumptions: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L552 -- that a newly-created provider has gen 0.
16:20:39 efried Because the POST /resource_providers responds 201 with no content.
16:21:48 edleafe efried: that's the same assumption as earlier. generation starts at 0, and each change increases it by one
16:21:59 edleafe if you just created it, it's at zero
16:22:19 efried edleafe Right. Which isn't a good assumption, unless it's a documented part of the API, which today it ain't.
16:22:47 edleafe It's sounding more and more like a doc bug
16:23:17 efried If in fact we want it to be part of the API. Which I'm not convinced of.
16:29:13 efried I'm gonna say we definitely have a doc shortage with respect to how to *use* the generation in any case. Other than that one spot in the API ref, there's not even anything that tells you you're supposed to send down the same generation you GET.
16:35:23 cdent efried, edleafe: I definitely agree (with dansmith) that _in the presence of the provider tree caching mechanism_ we should never gen += 1 on the client side. Instead we should always be prepared to invalidate the _entire_ cache when a 409 happens. This is the reason for creating the PlacementAPIConflict base class: so any gen conflict can cause a complete reset.
16:35:52 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: CLI for resource classes (v1.2) https://review.openstack.org/511182
16:35:53 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: RP list: member_of and resources parameters (v1.3, v1.4) https://review.openstack.org/511183
16:35:53 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: RP delete inventories (v1.5) https://review.openstack.org/514642
16:35:54 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: CLI for traits (v1.6) https://review.openstack.org/514643
16:35:54 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: Resource class set (v1.7) https://review.openstack.org/514644
16:35:55 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: Usages per project and user (v1.8, v1.9) https://review.openstack.org/514646
16:35:55 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: CLI allocation candidates (v1.10) https://review.openstack.org/514647
16:35:56 openstackgerrit Matt Riedemann proposed openstack/osc-placement master: [WIP] Get resource provider by uuid or name https://review.openstack.org/527791
16:36:05 jackie-truong mriedem: During the 1/4 meeting, you said that you weren't sure who should be reviewing the certificate validation feature and that you expect a lot of blueprints to get deferred. jaypipes has been reviewing the patch series since that meeting, but seems to be out this week. Do you think it's reasonable to try to get this patch series through before the feature freeze?
16:36:07 efried cdent No arguments there.
16:36:21 efried In the scenario at hand, we don't get a 409, even though we should.
16:36:28 cdent efried, edleafe; Another option is for the provider tree to do a full reset after any finished operation
16:36:34 edleafe cdent: so the cache will *always* be invalid, as we will never have agreement with the rp's data and its generation
16:36:42 mriedem jackie-truong: i'm not going to be able to context switch to an entirely new thing the day before FF
16:36:53 cdent edleafe: yeah, which is why I've always wondered why we have a cache?
16:36:54 edleafe cdnet: jinx-ish
16:37:04 efried cdent At some point you started work on the "updating generation when aggregates changed" - where is that?
16:37:11 edleafe cdent, not cdnet
16:37:26 efried cdent I thought you started writing a spec, but it must not have been in nova-specs.
16:37:28 mriedem jackie-truong: unfortunately it's probably going to get deferred to rocky
16:37:56 cdent efried: the scenario at hand is using logic that pre-dates the provider tree and was an acknowledged hack that was deemed safe-ish in the world of _not_ nested
16:38:02 mriedem jackie-truong: i'd like it if we could do some version of the 'slots' or 'runways' concept so these blueprints that keep getting deferred from release to release actually become a review priority early in the next cycle before we take on new work
16:38:21 cdent efried: mriedem told me that wasn't going to get considered until rocky, so I lowered it off my priority list in favor of rechecking lots of things over and over
16:38:32 cdent that == aggregates with generations
16:38:35 mriedem especially before the people grinding that contribution don't burn out and move on
16:38:43 jackie-truong mriedem: Yeah, that'd be nice
16:38:57 efried cdent Okay, did you not already start writing words about it, though?
16:39:01 mriedem jackie-truong: i'll add something to the dublin ptg agenda for discussion
16:39:05 jackie-truong mriedem: If I can find jaypipes, I'll see if he has any time to look at the patches. Otherwise, I'll plan to wait for rocky
16:39:37 efried cdent I think we need a launchpad-something (bug? blueprint?) for that.
16:40:01 efried cdent And I want to point to it from the bug I'm opening now, titled "Placement client cache consistency is broken"
16:40:12 cdent efried: https://blueprints.launchpad.net/nova/+spec/placement-aggregate-generation
16:40:14 edleafe cdent: did you see my comments starting here? http://p.anticdent.org/4liQ
16:40:40 edleafe That's my understanding how caching was supposed to work with generations
16:40:42 cdent efried: I hope you'll revise that to say ProviderTree cache consistency, not Placement. Placement is fine.
16:41:03 cdent edleafe: looking. I tried to read the whole backlog but am way sick, so faking having a brain
16:41:08 efried cdent "Placement client" not "Placement".
16:41:22 cdent it's not even a placement client, it's the provider tree
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 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.

Earlier   Later