Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-05-25
15:55:13 jaypipes dansmith: can you, in one sentence, explain your preference to change the RPC interface vs. add a field to the reqspec object?
15:55:21 sdague mriedem: done
15:55:35 jaypipes dansmith: I want to make sure I understand you completely.
15:56:00 dansmith jaypipes: we don't need to store the uuids in the database, right?
15:56:22 dansmith we've had this conversation a couple times
15:56:44 edleafe dansmith: but there is already the 'instance_uuid', which we don't store either
15:57:46 dansmith edleafe: um, what?
15:57:53 dansmith already instance_uuid in the reqspec?
15:58:00 edleafe yup
15:58:33 edleafe dansmith: https://review.openstack.org/#/c/463623/5/nova/objects/request_spec.py@77
15:58:38 dansmith what do you mean we don't store it
15:58:39 dansmith ?
15:58:49 dansmith ...right, I know it's in there
15:58:54 edleafe well, we don't *need* to store it
15:59:12 edleafe it's useless if num_instances > 1
15:59:15 dansmith okay not sure what that has to do with not adding another thing we don't need to store, which could be a hundred uuids :)
15:59:31 mriedem fuel on the fire makes it better right?
15:59:50 mriedem at what point do we add kitchen_sink to request spec?
16:00:21 edleafe I understand the reqspec to be the vehicle for passing the requested info to the scheduler
16:00:28 edleafe the scheduler needs this info
16:01:28 jaypipes dansmith: so the reason I was advocating passing in all the instance uuids in the reqspec was because I wanted to get rid of the retry stuff and do all of that in the scheduler (and thus we'd need all those UUIDs). But now that that is off the table, I'm not sure it makes sense any more.
16:01:50 dansmith aye
16:01:52 edleafe jaypipes: it definitely does
16:02:22 edleafe if num_instances > 1, how will the scheduler claim the resources if it doesn't know the consumer_id?
16:02:38 dansmith the uuid list is only something that makes sense for a single call to the scheduler, AFAIK, so it shouldn't be stuffed into the database, just something we pass like anything else, IMHO
16:03:19 jaypipes edleafe: a fair point.
16:03:52 jaypipes edleafe: if we moved the handling of num_instances > 1 to the conductor, though...
16:04:23 jaypipes edleafe: we'd need more robust concurrent update handling in the conductor, then.
16:04:24 edleafe jaypipes: sp the conductor would make num_instances calls to the scheduler, instead of 1?
16:04:32 jaypipes edleafe: I'm spitballing.
16:04:49 edleafe We'd also have to run the host queries num_instances times instead of once
16:05:05 jaypipes edleafe: right.
16:05:06 openstackgerrit Matt Riedemann proposed openstack/nova master: Exclude deleted service records when calling hypervisor statistics https://review.openstack.org/467220
16:05:16 edleafe you're spitballing; I'm poking holes
16:05:19 edleafe :)
16:05:24 mriedem Kevin_Zheng: ^ i updated the commit message and re-wrote the test a bit
16:05:25 jaypipes edleafe: I'm thinking out loud.
16:05:29 mriedem with a bunch of comments to explain it
16:05:45 edleafe me too
16:05:58 edleafe I've thought about these a lot already, though :)
16:06:07 sdague mriedem: I'm confused about your comment on https://review.openstack.org/#/c/467995/ about implications for versioned objects
16:06:28 sdague maybe that's where we need dansmith poking
16:07:39 openstackgerrit Ed Leafe proposed openstack/nova master: Pass the number of hosts to return explicitly https://review.openstack.org/465767
16:07:40 openstackgerrit Ed Leafe proposed openstack/nova master: Return multiple hosts from select_destinations https://review.openstack.org/465768
16:07:40 openstackgerrit Ed Leafe proposed openstack/nova master: Remove selecting host based on host_subset_size https://review.openstack.org/465174
16:08:01 edleafe jaypipes: ^^ The series is ready for review. I can switch the passing of UUIDs if needed
16:08:08 jaypipes edleafe, dansmith: for retries, do we ever retry the entire list of instances (in a num_instances > 1 case) or do we just retry the launch of one of them?
16:08:36 edleafe jaypipes: if we retry the whole list, that would be very dumb
16:08:52 edleafe since some could have already been successful
16:09:00 jaypipes edleafe: well, it all depends on how "atomic" we view the num_instances > 1 case, right?
16:10:05 dansmith jaypipes: we've lost scope of the others by the time we're retrying any of them
16:10:11 dansmith jaypipes: and they might've scattered across cells
16:10:23 dansmith jaypipes: as you've pointed out, the api implies atomicity, and we guarantee almost none
16:10:25 mriedem sdague: yes, that's my call for dan
16:11:01 edleafe the min_count and max_count were supposed to provide a go/no-go atomicity
16:12:54 sdague mriedem: ok, cool. Jenkins is currently a happy camper, so there is at least that
16:13:38 jaypipes dansmith, edleafe: ok, random idea here... what if we triggered claims in the scheduler only if num_instances == 1? at least for right now... and let the num_instances > 1 case be handled in the same wonky claim-on-compute way it is now?
16:13:56 dansmith oh please god no
16:14:03 cdent what would that gain?
16:14:12 dansmith jaypipes: nobody wants to claim on compute right?
16:14:31 jaypipes it would mean we would solve the 80% case now. and come up with a solution for the 20% case later.
16:14:34 edleafe jaypipes: yeah, that sounds terrible
16:15:04 edleafe the only thing missing to solve 100% of the claims is the instance UUIDs.
16:15:20 edleafe As long as we pass those in some form to the scheduler, we are good
16:15:38 edleafe the only question is what is the best way to pass them
16:15:47 jaypipes edleafe: right, but what I'm saying is we solve that later and for the first patch(es), we bite off what we can chew for right now.
16:16:00 jaypipes instead of trying to solve everything at once.
16:16:17 edleafe IMO the patches are bite-sized
16:16:42 jaypipes edleafe: the patches might be. the debate isn't. :)
16:16:50 mriedem pass them over rpc
16:16:54 mriedem don't store them in the db
16:16:58 mriedem i thought that was clear?
16:17:07 dansmith mriedem: me too
16:17:10 jaypipes mriedem: nobody has suggested storing them in the DB.
16:17:13 edleafe mriedem: well, if that's the decision, then everything is ready for review
16:17:18 dansmith jaypipes: perhaps we can nail this down quicker in a hangout?
16:17:22 dansmith jaypipes: edleafe did :)
16:17:28 jaypipes he did?
16:17:31 mriedem if you put them in the reqspec they go in the db unless you monkey the object to not persist
16:17:35 mriedem but that seems wrong
16:17:39 edleafe I suggested putting them in the reqspec
16:17:51 jaypipes ok.
16:17:54 edleafe so I guess DB storage is inevitable :)
16:17:59 mriedem i mean sure you can throw any attribute on an object and it won't persist
16:18:07 mriedem if it's not in the model
16:18:09 jaypipes right
16:18:12 edleafe jaypipes: to be fair, you suggested the same thing :)
16:18:18 jaypipes that's what I thought we were going to do..
16:18:24 edleafe me too
16:18:40 jaypipes edleafe: not store in the DB. I suggested just tacking instance_uuids as a field on the object, since that would avoid an RPC API change.
16:18:43 mriedem it seems very not intuitive
16:18:51 dansmith jaypipes: it doesn't avoid an rpc change
16:18:55 mriedem 2 years from now someone is going to ask, wtf, why are these on this object but not in the db?
16:18:59 dansmith yeah
16:19:03 dansmith it's silly to do that
16:19:16 cdent there's nothing wrong with an rpc api change, and it is semantically clean. let's just do that.
16:19:17 jaypipes fine. call me silly. I don't care.
16:19:33 cdent "that" == add a param
16:19:33 edleafe I've done it both ways, so I don't care either :)
16:19:34 dansmith jaypipes: you know you're just generally silly anyway :)

Earlier   Later