Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-24
15:04:58 cdent efried: I tried to do another recheck run through your changes this morning (and some others) but I'm not sure how much impact it had
15:05:43 efried cdent Seen and appreciated sir. Silver lining: it takes *some* of the pressure off getting the top of the series perfect.
15:06:03 efried At this rate, even things +W'd by FF will take a week to merge.
15:09:22 openstackgerrit Alex Xu proposed openstack/nova master: placement: using the dict format for the allocation in claim_resources https://review.openstack.org/536083
15:09:23 openstackgerrit Alex Xu proposed openstack/nova master: Fix nits in support traits on allocation candidates API https://review.openstack.org/537351
15:09:23 openstackgerrit Alex Xu proposed openstack/nova master: placement: enable required traits from the flavor extra specs https://review.openstack.org/536085
15:09:56 alex_xu edleafe: mriedem stephenfin, thanks for the review, ^ addressed all the comments
15:11:55 edleafe alex_xu: thanks - will review soon
15:12:07 alex_xu edleafe: thanks
15:12:44 mriedem alex_xu: were you going to put up a patch for the placement api-ref docs changes? https://github.com/openstack/nova/blob/master/placement-api-ref/source/allocation_candidates.inc#L27
15:13:11 efried cdent mriedem Sorry for not understanding how placement version lockstepping works; are we now ready to address these comments? https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L902-L904 https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1117-L1119
15:13:45 cdent efried: https://review.openstack.org/#/c/528794/
15:14:07 cdent that may be out of date by now, since it's been sitting there for quite a while
15:14:16 efried cdent Thanks, at least it's on the radar.
15:15:23 mriedem efried: yeah we require >=1.14 now really, or that's what nova-status is checking for and the compute requires for root providers,
15:15:33 mriedem efried: but at this point might as well hold that off until rocky - the cleanup i mean
15:16:24 mriedem dansmith: that reminds me - we havent bumped major compute rpc api versions in a few years; that's historically something you've been the master of, are you interested in doing one of those, since i think it has to happen shortly before RC1 yeah?
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 '_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.

Earlier   Later