Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-24
15:16:51 mriedem we have a lot of "drop this compat code when we bump major versions"
15:17:01 efried okay. I must also not have a good handle on what kind of patch will be accepted after FF. This isn't a feature, so...?
15:17:49 dansmith mriedem: yeah I should probably do that
15:20:16 mriedem Roamer`: looking at the latest cells job failure on your patch http://logs.openstack.org/33/140733/19/check/legacy-tempest-dsvm-cells/3e2f79f/
15:20:22 mriedem it's timing out waiting for a volume backup to complete,
15:20:31 mriedem looking at the node it's running on: http://logs.openstack.org/33/140733/19/check/legacy-tempest-dsvm-cells/3e2f79f/zuul-info/inventory.yaml
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 '_to_dict' functions https://review.openstack.org/537413
15:23:15 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Remove legacy '_from_dict' functions https://review.openstack.org/537414
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 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: remove pagesize from __init__ of InstanceNUMATopology https://review.openstack.org/485553
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:11 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: remove related pinning from __init__ of InstanceNUMATopology https://review.openstack.org/485554
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: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 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:00 efried edleafe Sorry, right, add 1 to all the gen numbers after the first one.
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 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:39 dansmith bauzas: are you around today to address my minor comments here? https://review.openstack.org/#/c/535693/5
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:

Earlier   Later