Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-09-07
15:13:54 mriedem bauzas: i prefer an rpc parameter to select_destinations rather than hiding things in the already super confusing RequestSpec object
15:14:19 mriedem because force_hosts, requested_destination, and then a forced_destination would be even more complicate
15:14:20 mriedem *complicated
15:14:34 mriedem skip_filters in the rpc api parameters is pretty clear
15:18:42 gibi cores: there is a notification transformation patch only needs a second +2 if you are bored https://review.openstack.org/#/c/417882/
15:20:17 bauzas mriedem: I tend to prefer passing versioned objects than parameters on RPC APIs because I think it's clearer, you know
15:20:30 bauzas mriedem: the fact is that I agree with you, we should get rid of forced_hosts
15:20:52 bauzas mriedem: but I was just proposing to only use 2 fields : one for requesting the scheduler filters, one for not
15:21:02 bauzas that could be a comment in the object
15:21:17 mriedem i guess it will probably come down to whether or not we need to persist that information for later
15:21:26 bauzas we already have a long list of explicit parameters in the API method, and I'm not sure it's cool
15:21:40 bauzas mriedem: we have an helper method for that in the object
15:21:51 bauzas mriedem: and there are fields that aren't persisted already
15:22:08 bauzas so, maybe the work consists of making the line clearer in between what requires to be persisted and what's not
15:22:24 bauzas I could do that
15:22:31 stephenfin sdague: Nice little doc cleanup here, if you're interested https://review.openstack.org/#/c/501342/
15:23:33 mriedem def select_destinations(self, ctxt, spec_obj, instance_uuids):
15:23:36 mriedem bauzas: ^ is a long list?
15:24:42 bauzas mriedem: you know we had a problem with instance_uuids being different from what we have in the spec object, so I'd say one parameter is maybe too much
15:25:16 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove dest node allocation if evacuate MoveClaim fails https://review.openstack.org/499878
15:25:30 bauzas mriedem: the real crux of the problem is that I never explicited whether the RequestSpec object you pass is either the original spec or the amended spec in case of a move
15:26:01 mriedem gdi now my git review push failed
15:26:20 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove dest node allocation if evacuate MoveClaim fails https://review.openstack.org/499878
15:26:20 openstackgerrit Matt Riedemann proposed openstack/nova master: Pass migration from API to conductor for evacuate https://review.openstack.org/500176
15:26:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Add a test to make sure failed evacuate cleans up dest allocation https://review.openstack.org/499877
15:26:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Add recreate test for evacuate claim failure https://review.openstack.org/499874
15:26:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Create allocations against forced dest host during evacuate https://review.openstack.org/499399
15:26:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Refactor out claim_resources_on_destination into a utility https://review.openstack.org/499718
15:26:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Modernize set_vm_state_and_notify https://review.openstack.org/499799
15:26:39 bauzas do folks agree with the fact that we should disable reschedules if someone is requesting a destination when cold migrating ?
15:26:43 mriedem the bottom change there is going to need to be re-approved
15:26:54 bauzas mriedem: ack will do
15:27:31 mriedem don't we already restrict reschedules like that in some other case?
15:28:17 bauzas mriedem: we don't reschedule for both evacuate and live-migrate
15:28:28 bauzas we only reschedule for two cases : boot and resize
15:28:38 mriedem well, we do reschedule for live migrate
15:28:40 mriedem from conductor
15:28:42 mriedem if the pre-checks fails
15:28:43 bauzas well, and cold migrate since resize shares the same codepath
15:28:43 mriedem *fail
15:29:00 bauzas mriedem: you mean at the conductor level ?
15:29:04 mriedem yes
15:29:09 bauzas mriedem: ah yeah
15:29:12 bauzas that's recent
15:29:41 bauzas I was talking of the classical reschedule situation where a compute raises an exception and then calls out conductor by amending the retry dict
15:29:44 mriedem evacuate in the conductor doesn't reschedule
15:30:17 mriedem if you specify a host for cold migrate and that host fails, we shouldn't reschedule IMO
15:30:39 bauzas the situation I mention in takashi's spec is that if you allow 3 reschedules by default, you could have the scenario of a requested destination call, and then if the compute fails, then ending up finding another compute node
15:30:48 bauzas mriedem: that's my point
15:30:58 mriedem yes i realize, it's ComputeManager._reschedule_resize_or_reraise
15:31:02 bauzas I just wanted to make sure we all agree
15:32:21 mriedem now, live migration allows you to specify a host, not force it, and it will retry migrate_max_retries times
15:32:25 mriedem which defaults to -1 for unlimited
15:34:03 mriedem although i think in the case of live migration, if you specify a host (without force) and the scheduler raises NoValidHost, we're done
15:34:06 mriedem there is no retry
15:34:33 mriedem otherwise if you don't specify a host, we just keep trying until we find a host or run out of hosts and the scheduler raises NoValidHost
15:34:33 bauzas that's fine then
15:34:39 bauzas for live migrate
15:35:10 mriedem yeah so for cold migrate if i specify a host and the scheduler kicks it out with NoValidHost, you're done
15:35:22 mriedem if the scheduler says it's ok and the compute fails, we don't reschedule to another host via prep_resize
15:35:24 mriedem you're done
15:35:28 bauzas okay, so I +1d the spec
15:35:41 bauzas once takashi amends the spec for mentioning that, I +2
15:35:47 mriedem did he remove the stuff about adding the force flag to the cold migrate api?
15:35:52 bauzas he deserves more attention than last cycle :)
15:35:55 bauzas he did
15:36:19 mriedem well, what i'd also like to see with this is to not do the same underhanded request spec stuff that live migrate and evacuate do when a host is specified,
15:36:31 mriedem i.e. the API does some logic and shoves stuff in the request spec, which passes through conductor to the scheduler
15:36:47 mriedem i'd prefer if that was all more explicit between api->conductor-scheduler
15:37:07 mriedem because whenever i look at the conductor code i have to know exactly how the api code works
15:38:09 bauzas mriedem: when I wrote the original blueprint, there was a reason
15:38:45 bauzas mriedem: I necessarly needed to set the Spec fields by the API because the force or not force logic is there
15:39:22 bauzas mriedem: plus the fact that you can call again the conductor by the compute if you resize
15:39:48 bauzas mriedem: those reasons led me to set the fields there in the API
15:40:11 bauzas but I agree with the fact that is spaghetti code
15:42:06 mriedem i've tried to document the hell out of all of these areas that i've had to touch for live migration and evacuate to help clarify some of it
15:42:19 mriedem but we still need some detailed documentation in the request spec object itself per my ML email
15:42:40 mriedem plus the fact that we don't convert some things to primitive in the request spec
15:42:42 mriedem which seems like a bug
15:43:09 mriedem but, i don't really know enough about it or if it's something we should care about
15:43:25 mriedem i.e. if you only have to convert to a primitive for legacy stuff due to an old scheduler client, then we probably don't care
15:45:52 bauzas when you say primitives, you mean legacy dictionaries ?
15:46:17 bauzas not the primitived dictionary in terms of o.vo ?
15:46:55 bauzas honestly, the problem is that we still have lots of back-and-forths between those legacy dicts and the new object, which is errorprone
15:47:24 bauzas so, my goal is to reduce those by changing the scheduler helper methods to accept a spec object
15:51:20 mriedem the legacy dict
15:53:36 mriedem gibi: in https://review.openstack.org/#/c/417882/23/doc/notification_samples/instance-resize-error.json it looks weird in a few places where there are empty dicts or lists and there is a blank line in between
15:54:22 bauzas by reading the news, all my thoughts go to folks in Florida, Georgia and South Carolina. Hope jaypipes will be all good
15:54:49 jaypipes bauzas: it's a logistical nightmare :(
15:55:18 bauzas jaypipes: you're far from me, buddy so I can hardly help
15:55:29 jaypipes bauzas: heh, I know :) it's cool, dude.
15:55:35 bauzas jaypipes: but in case you need anything I can do, just lemme know
15:55:41 jaypipes bauzas: thx :)
15:56:12 bauzas jaypipes: you're temporarly relocating, as you said ?
15:57:38 jaypipes bauzas: yeah, though plans have changed... now looking to got to Asheville, NC for a night, drop the girls off with my parents and then catch flights out of ATL.
15:57:47 jaypipes bauzas: it's a clusterf**k
15:59:20 bauzas :(
16:17:10 openstackgerrit Stephen Finucane proposed openstack/nova master: conf: Remove 'vendordata_driver' opt https://review.openstack.org/397835
16:17:14 openstackgerrit John Garbutt proposed openstack/nova master: Enable test_iscsi_volume in live migration job https://review.openstack.org/459316
16:17:36 stephenfin mriedem: Per mikal's comment earlier, could you remove the -2 on this, please? https://review.openstack.org/397835

Earlier   Later