| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-24 | |||
| 15:20:38 | mriedem | provider: rax-dfw | |
| 15:20:52 | mriedem | those have been really really slow since the intel kernel patcharoo | |
| 15:22:52 | openstackgerrit | Merged openstack/osc-placement master: CLI for aggregates (v1.1) https://review.openstack.org/505643 | |
| 15:23:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Remove 'NUMATopologyLimits.obj_from_db_obj' https://review.openstack.org/537412 | |
| 15:23:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Remove legacy '_from_dict' functions https://review.openstack.org/537414 | |
| 15:23:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Remove legacy '_to_dict' functions https://review.openstack.org/537413 | |
| 15:23:54 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Transform instance.resize_prep notification https://review.openstack.org/465081 | |
| 15:27:52 | edmondsw_ | gibi here's your requested daily reminder to look at https://review.openstack.org/#/c/526094/ if you get a chance :) | |
| 15:33:54 | gibi | edmondsw_: thanks :) | |
| 15:34:09 | efried | cdent edleafe Not loving the fact that DELETE RP inventory returns 204 and the client code is assuming the RP generation can just be incremented by one. | |
| 15:34:48 | cdent | if you've deleted an RP why would you need to know it's generation? | |
| 15:34:49 | cdent | it's gone | |
| 15:34:55 | openstackgerrit | Merged openstack/osc-placement master: Add missing runtime requirements https://review.openstack.org/536870 | |
| 15:35:06 | efried | cdent Deleted its inventory, not the RP itself. | |
| 15:35:26 | edleafe | efried: what would you love more? | |
| 15:35:30 | cdent | then GET the /rp/{uuid}, that's what it's there for | |
| 15:36:09 | cdent | this notion that it is bad to make requests is unproven. just make requests | |
| 15:36:24 | efried | edleafe Either DELETE returns a payload, which would look like { 'resource_provider_generation': N, 'inventories': {} }, or we always use PUT with {} | |
| 15:36:51 | efried | ...which does that | |
| 15:37:11 | efried | cdent It's not quite that simple. | |
| 15:37:26 | openstackgerrit | Merged openstack/osc-placement master: Address comments from original inventory patch https://review.openstack.org/521578 | |
| 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. | |