Earlier  
Posted Nick Remark
#openstack-nova - 2018-01-24
14:53:45 mriedem https://launchpad.net/ubuntu/+source/python2.7
14:53:47 mriedem jroll: ^
14:53:56 mriedem https://launchpad.net/ubuntu/+source/python2.7/2.7.12-1ubuntu0~16.04.3
14:54:00 mriedem released 1/18
14:55:36 mriedem should compare the versions of the python2.7 package in the failing ironic CI jobs
14:57:16 mriedem bauzas: thanks for hitting those osc-placement changes
14:59:48 efried rgerganov So as written, this test will fail?
14:59:54 rgerganov efried, yes
15:01:33 efried rgerganov Good deal. If you don't mind, as I work through this today, I think I'm going to incorporate your test and (some version of) your fix into the update_from_provider_tree patch. You'll get co-author credit, of course :)
15:02:04 rgerganov efried, fine with me :)
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: placement: enable required traits from the flavor extra specs https://review.openstack.org/536085
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: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 '_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]

Earlier   Later