Posted	Nick	Remark
#openstack-nova - 2018-01-24
15:37:27	efried	cdent The point of getting the generation in the response is that I know it's the generation right after the inventory was deleted.
15:37:34	openstackgerrit	Matt Riedemann proposed openstack/osc-placement master: Usage docs and initial release note for osc-placement https://review.openstack.org/536858
15:37:37	mriedem	stephenfin: done ^
15:37:44	cdent	If you are depenendent on that, you're not rally making a very resilient system?
15:37:46	efried	cdent Whereas if I do a subsequent GET, I don't know if an intervening operation incremented the generation as well.
15:37:55	cdent	yes that's _always_ the case
15:38:01	cdent	whether you did the the GET or not
15:38:11	efried	cdent No, not when I get the generation in the response.
15:38:17	cdent	you have no idea, ever, what any other client or thread might be doing with the same resource provider
15:38:37	Roamer`	mriedem, yes, I saw it was rax-dfw on the previous runs, too, that's why I haven't complained to -infra (I guessed somebody else with more authority would complain again sooner or later), but thanks for raising it in -infra right now
15:38:41	cdent	when you get the generation in the response, something else, immediately, might have changed it
15:38:47	cdent	that's always true
15:38:55	cdent	what ever mechanism you are using to track generations
15:38:59	efried	cdent Yes, but I don't need to care about that until the next time I go to update something.
15:39:12	cdent	which should always be the case
15:39:21	cdent	you should always be prepared to get a 409
15:39:30	cdent	and act accordingly
15:39:39	edleafe	gotta agree - I don't see what getting the generation in the response buys you
15:39:49	efried	I need to be able to get a 409 reliably, though.
15:39:51	efried	Look look.
15:39:57	cdent	the generation should be utterly meaningless
15:40:06	cdent	sorry, I gotta go fetch the cat, will catch up in 30'
15:40:08	efried	That's exactly why I shouldn't be assuming it gets incremented by 1
15:40:18	efried	(the meaningless thing, not the cat thing)
15:40:37	edleafe	if the delete succeeds, then it will be incremented by 1. You can certainly assume that
15:40:41	mriedem	Roamer`: see -cinder
15:40:46	Roamer`	mriedem, yes, looking
15:40:50	efried	edleafe No way.
15:40:51	Roamer`	I mean watching
15:40:54	mriedem	looks like cinder made a change in the last 24 hours that is slowing down volume backup
15:41:01	edleafe	What you can't assume is that the next time you do something, it won't have been changed by something else
15:41:07	efried	edleafe That's not part of the contract. It just happens to be the way placement is implemented.
15:41:23	efried	This is exactly my point
15:41:24	Roamer`	mriedem, ah, you even figured out which one! I just mentioned it a couple of times there, but it seemed nobody was awake yet...
15:41:57	efried	It is crucial that the generation I have in my local cache matches the data I have in my local cache.
15:42:03	edleafe	efried: do you mean if generation was a hash of the current state, you wouldn't be able to make any assumption?
15:42:24	efried	edleafe Or any other implementation that's not a monotonically increasing integer.
15:42:52	efried	edleafe I mean that the format of the generation is not part of the API contract at all, and the client side should be assuming absolutely nothing about it.
15:43:07	edleafe	So what would getting the generation buy you? It *still* could have been changed by the time you go to use it
15:43:35	efried	Yes, in which case I will legitimately get a 409 and need to refresh/redrive my change. That's the good path.
15:43:41	efried	The bad path is this:
15:45:07	edleafe	efried: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/schemas/inventory.py#L20-L25
15:45:53	efried	I have generation 5. I do my DELETE. Then you do a GET (so you have generation 5) and a PUT to create some inventory. You're now at gen 6. Now I do my GET as cdent suggested and find out the gen is 6. So I think gen 6 corresponds with empty inventory when in fact it does not. Now I decide to add some inventory back. I do a PUT with that new inventory and gen 6, which will *succeed* because the server has gen 6. But
15:45:54	efried	I just blew away your inventory update.
15:46:10	mriedem	Roamer`: https://bugs.launchpad.net/cinder/+bug/1745168 for rechecks i guess
15:46:11	openstack	Launchpad bug 1745168 in Cinder "volume backup tests timing out since 1/23" [Undecided,New]
15:46:21	mriedem	melwitt: dansmith: et al ^ if you see gate timeouts
15:46:31	dansmith	ack
15:47:10	efried	edleafe Great, it's defined as an integer, in the schema, for the current microversion. There's still nothing in the contract that tells me it's monotonically increasing starting from zero.
15:47:10	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: remove pagesize from __init__ of InstanceNUMATopology https://review.openstack.org/485553
15:47:11	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: remove cpuset_reserved from __init__ of InstanceNUMATopology https://review.openstack.org/466030
15:47:11	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: remove related pinning from __init__ of InstanceNUMATopology https://review.openstack.org/485554
15:47:22	edleafe	efried: "Then you do a GET (so you have generation 5)" - that's not correct. You'd get 6 in that case, since the DELETE succeeded
15:48:00	efried	edleafe Sorry, right, add 1 to all the gen numbers after the first one.
15:48:00	Roamer`	mriedem, thanks, I'll use it if needed... let's hope it's not needed much longer, because there's not much time left till... what, tomorrow? :)
15:48:11	efried	edleafe Or whatever. The logic is the same.
15:48:33	mriedem	Roamer`: eod tomorrow yeah
15:48:46	edleafe	efried: so you'd think you're at 6, but meanwhile someone did an update to move it to 7. You then PUT the inventory at 6 (since you don't know it's been changed), and get a 409
15:49:01	edleafe	Nothing blown away
15:49:16	Roamer`	mriedem, aaaaand if failed again.... and I can't do a recheck before it finishes failing...
15:49:22	efried	edleafe No, I don't think I'm at 6 unless I assume the generation increments by 1
15:49:39	dansmith	bauzas: are you around today to address my minor comments here? https://review.openstack.org/#/c/535693/5
15:49:39	efried	edleafe I'll think I'm at 7 because I did my GET (to discover the "correct" generation) after you did your PUT.
15:49:53	dansmith	bauzas: if not I can do it, but I'd like to retain my impartiality to be able to vote on it :)
15:50:04	edleafe	efried: Huh? I thought you said "Then you do a GET" after the delete
15:50:11	mriedem	Roamer`: i'll recheck it throughout the day
15:50:14	edleafe	Where are you assuming anything
15:50:17	edleafe	?
15:50:36	efried	edleafe Just so.
15:50:44	Roamer`	mriedem, so yeah, I'll keep on rechecking... oh? You will? Thanks a lot! (again... it seems that I'm thanking you on every other line...)
15:51:06	efried	edleafe If I'm not assuming the generation increments by 1, and I have to do a GET to discover what the generation is after I do my DELETE, that's broken.
15:51:48	edleafe	efried: I still don't see the issue
15:52:01	efried	Here it is again with the correct generations:
15:52:28	edleafe	efried: you can make the assumption, and if something else changes it, you get a 409 and *then* do the GET to see what the current state is
15:52:30	efried	ause the server has gen 7. But I just blew away your inventory update.
15:52:30	efried	I have generation 5. I do my DELETE. Server has gen 6, I have gen 5 still. Then you do a GET (so you have generation 6) and a PUT to create some inventory. You and server are now at gen 7. Now I do my GET and find out the gen is 7. So I think gen 7 corresponds with empty inventory when in fact it does not. Now I decide to add some inventory back. I do a PUT with that new inventory and gen 7, which will *succeed* bec
15:53:33	efried	edleafe Perhaps the disconnect is where I'm doing that GET to discover the generation. At that point I'm only doing a GET /resource_provider/{uuid} - I really *don't* want to go getting its traits, aggs, and inventories as well.
15:53:45	edleafe	efried: if you do a GET and ignore that there is now inventory and then proceed to blow it away, that's not placement's fault
15:54:00	efried	Then there's no point having a cache.
15:54:43	efried	We have this cache that we're using successfully for most of the operations, but if we run any DELETE API, we have to blow it away and start over? That seems not okay.
15:54:47	edleafe	efried: especially if you have two different systems claiming to be authoritative on what the inventory for an RP is
15:55:35	efried	That's the entire reason we have this generation business in the first place. To manage sync issues when more than one thread claims authority over a provider.
15:56:14	efried	And I'm not saying it's placement's fault that this happens. It's a matter of cache management by the client. Which IMO we're not doing correctly in this scenario.
15:56:49	edleafe	efried: yes, but it is to prevent *accidental* conflicts. If you know you have a conflict and proceed ahead anyway, there isn't a system that will make that OK
15:57:01	efried	But I don't know I have a conflict.
15:57:14	efried	Put another way, a client shouldn't use DELETE for inventories (or traits or aggs if it comes to that) if there's no response containing the generation.
15:57:55	edleafe	efried: your cache has RP, generation, and inventory. You go to update that and get a 409. It is now important that you invalidate the cache before you proceed
15:58:17	efried	I don't get a 409. That's the crux of the problem.
15:58:40	edleafe	efried: you don't do the GET *before* trying to update
15:59:12	efried	How else do I discover the "new" generation?
15:59:28	efried	(the one I think is post-DELETE, but which is in fact post-my-DELETE-and-your-PUT)
15:59:32	edleafe	You update based on what the cache says. If the cache is current, the generation it has will work. If something else has changed in placement, you invalidate the cache, and do a GET to refresh it
15:59:51	efried	and how do I know "something else has changed in placement"?
16:00:02	edleafe	that's where the assumption that after the delete, the generation is original gen + 1
16:00:38	efried	Right. Which works, today, because we're dirtily exploiting what we know to be the internals of the placement service.
16:01:25	edleafe	"dirtily" - heh
16:01:29	edleafe	it's documented
16:01:44	edleafe	Give me a second to write up the flow as I understand it.
16:01:52	efried	edleafe Where is it documented?
16:04:49	efried	edleafe In the API ref, all we get is: "A consistent view marker that assists with the management of concurrent resource provider updates." and "409 Conflict if the resource_provider_generation doesn’t match with the server side." <== note "match"

Earlier   Later