Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-09-07
14:43:15 mriedem ok
14:43:48 mriedem i have to modify something in the 2nd to bottom patch for sylvain so i was waiting to do that
14:47:35 gibi I'm OK with the first 4 patches in that series and just started reading the 5th
14:55:46 dtantsur hi folks! I'm afraid to imagine how often you've heard this question, but.. what's the difference between nova migrate and nova evacuate?
14:55:54 dtantsur I'm figuring out which one fits better into https://review.openstack.org/449155
14:56:50 edleafe dtantsur: http://www.danplanet.com/blog/2016/03/03/evacuate-in-nova-one-command-to-confuse-us-all/
14:57:11 dtantsur thanks!
14:58:00 edleafe dtantsur: I keep that one handy, because after all these years it's still confusing
15:00:07 openstackgerrit Stephen Finucane proposed openstack/nova master: doc: Add documentation for emulator_thread_policy https://review.openstack.org/501721
15:00:55 stephenfin sahid, bauzas, gibi: ^ addressed sahid's comments
15:00:58 stephenfin I think...
15:03:12 gibi stephenfin: I think so too
15:04:15 bauzas stephenfin: +Wiiii
15:08:49 bauzas mriedem: so, 2 points
15:09:11 bauzas mriedem: just remove the upgrade reno and then I'll +2 https://review.openstack.org/#/c/499399/6
15:09:41 bauzas mriedem: also, about your skip_filters thoughts, just remember that we already have something called force_hosts
15:09:51 bauzas of course, that field is terribly named
15:10:22 bauzas but we could just not add yet again a new RPC API parameter, and rather just use the RequestSpec object for that
15:10:25 gibi stephenfin: I made an answer to your comment in https://review.openstack.org/#/c/463946/9/nova/tests/functional/notification_sample_tests/test_keypair.py@20
15:10:34 openstackgerrit Merged openstack/nova-specs master: Convert consoles code to use objects framework https://review.openstack.org/500975
15:11:03 bauzas mriedem: like, we could create a new field (and deprecate the legacy one) called forced_destination
15:11:12 bauzas having a Destination value
15:11:37 bauzas and so, if you pass a forced host, then the scheduler would claim
15:13:06 mriedem efried: ok is the first half of wednesday to your liking? https://etherpad.openstack.org/p/nova-ptg-queens
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

Earlier   Later