Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-09
16:16:44 dansmith people have to run this at various times or things won't work
16:17:09 imacdonn that may not be immediately obvious
16:17:41 imacdonn I've upgraded at least pike -> queens -> rocky without doing any online migrations, and nothing obviously didn't work
16:18:09 dansmith we have some db migrations which have blocked if you haven't run these to completion
16:18:14 dansmith maybe none since pike, but..
16:18:17 mriedem http://git.openstack.org/cgit/openstack/openstack-ansible-os_nova/tree/tasks/nova_db_setup.yml#n98
16:18:25 mriedem osa is certainly using it
16:18:51 dansmith I guess the default now is to run until completion, which is probably what people are doing I guess
16:19:09 dansmith but I know the return value here was critical earlier when people were running batches themselves
16:19:24 mriedem imacdonn: we also migrate some stuff online outside of the command
16:19:29 mriedem like on read from the db
16:19:32 mriedem or new resource create
16:20:16 imacdonn yeah, I know ... my point is that it's possible to get away without running the command, at least in some circumstances
16:20:41 dansmith imacdonn: I'm not sure what that has to do with anything
16:20:53 dansmith OSA and tripleo, and I expect other systems run this explicitly
16:20:55 mriedem if it's possible it's by chance
16:21:09 dansmith if you don't and it doesn't break in the versions you use, then you got lucky,
16:21:13 mriedem like dan said, we probably just haven't had a blocker migration in awhile
16:21:17 dansmith but that doesn't really mean anything for how important this is to notbreak
16:21:30 mriedem also depends on how old your data is,
16:21:50 mriedem i plan on dropping our request spec compat from newton in stein and if you don't have that migration done you'll fail to do things like migrate instances
16:21:53 imacdonn OK, nevermind .. I wasn't disgreeing that it's important to solve ...
16:22:17 mriedem so just make this return 2, doc and reno it and we're happy right?
16:23:02 imacdonn yeah, that'd work for this particular problem ... although it's probably not backportable ?
16:23:20 imacdonn (since it'll break things that don't know to check for 2)
16:23:58 mriedem i'm not sure; if things are failing but tooling is not aware of it, i think it's probably better to opt to the side of putting an upgrade release note and saying this will fail now
16:24:08 mriedem but i'd rather know something isn't working explicitly
16:24:19 mriedem dansmith: agree? ^
16:24:31 imacdonn but if the automation is just repeating infinitely until it gets a 0, it'll .... repeat infinitely
16:25:33 dansmith what if 2 means "I didn't do anything but there were exceptions", 1 means "I did things, maybe there were some exceptions too", 0 means "I didn't do anything, but no errors"
16:25:48 dansmith repeat on 1, done on 0, 2 means we hit terminal fail state
16:26:24 dansmith either way people that loop on nonzero will break with anything you're going to do, which is why reno and doc is super important
16:26:39 imacdonn right
16:27:37 dansmith not sure how I feel about changing behavior in a backport with a retroactive reno, but mriedem is the authority here, so I'd do whatever he says
16:28:21 imacdonn I'm thinking that most people only read release notes for a new release, not for errata updates
16:28:43 dansmith unfortunately I don't think they even read them for new releases, but.. yeah
16:28:49 imacdonn heh yeah, there is that
16:30:25 mriedem i'll defer to tonyb
16:31:21 dansmith the AUD stops with tonyb
16:36:50 melwitt .
16:43:21 mriedem i get it
16:43:24 mriedem took me awhile
16:48:42 imacdonn dansmith: I'm on the fence about your last suggestion .... I think I tend towards exceptions being bad, requiring some problem to be addressed immediately ... but then there was a suggestion that some migrations may raise exceptions "by design" if some other migration had not yet been completed
16:49:26 dansmith imacdonn: well, by design or not, we've had some that won't complete until others do
16:55:09 imacdonn dansmith: is it defined somewhere that a migration should raise an exception in such a case? (as opposed to just not doing any work) Seems like ideally there should be a way for a migration to explicitly state that "I can't do this *yet*"
16:55:39 dansmith the way to do that is to return nonzero found, with zero done
16:56:02 imacdonn so how do you distinguish that from "I can't do this *all all, ever*" ?
16:56:20 dansmith regardless, because of the complexity of hitting all the cases of live data, which we're historically bad at doing, making the process stop on exception is just practically not the best plan, IMHO
16:56:52 dansmith there's no case where found is nonzero where done is expected to remain zero forever
16:56:58 dansmith found is items that should be migratable
16:59:51 imacdonn OK
17:00:05 imacdonn I'll try to implement that and see what falls out
18:30:39 sean-k-mooney melwitt: i left some feedback in https://review.openstack.org/#/c/575735/2 fyi. hope that helps. the code should work but its duplicating logic that is not needed.
18:31:46 melwitt sean-k-mooney: cool thanks
18:43:52 mriedem dansmith: off the top of your head, do you know much about the migration_context we stash on the instance during cold migration / resize (created by the RT move claim) and what we need it for besides routing neutron events to the source and dest host? looks like it's otherwise for tracking numa/pci on the source and dest host,
18:44:21 mriedem reason i ask is because i'm currently using a move claim for the cross-cell resize but i have alternatives to using a resize_claim,
18:44:29 mriedem both claim ways in the RT are kind of weird for how i'm doing this
18:46:31 mriedem re: https://review.openstack.org/#/c/603930/9/nova/compute/resource_tracker.py and https://review.openstack.org/#/c/603930/9/nova/compute/manager.py@5138
18:49:49 mriedem i don't think i need to care about the migration_context for the same reasons as normal resize because for cross-cell, we'll have shelved offloaded from the source by the time we claim on the dest, so meh
18:50:07 dansmith mriedem: well, the point of it was to avoid doing things like looking up the most recent unfinished migration for an instance in order to get at things we were going to stash on there
18:50:19 dansmith probably for things that aren't covered by the old/new flavor, so yeah probably numaish things
18:50:42 dansmith maybe, but I guess I'd hope that we could make it as similar of a process as possible,
18:50:58 dansmith and the fact that we only have migration-context for cold moves right now is unfortunate I think
18:51:22 mriedem well, we can make it similar, but things get weird if we do, as noted in those links above
18:52:10 mriedem i just haven't ran into anything with this that requires needing the migration_context being set on the instance
18:52:52 dansmith we use it for directing notifications to both computes right?
18:53:32 mriedem yes, but i don't really need that with cross-cell resize,
18:53:44 mriedem because sending an event to the source is useless b/c we've shelved offloaded from the source
18:54:05 mriedem when we unshelve on the target, the instance.host gets set and the API will route the event there
18:54:05 dansmith sure, I was just responding to "we don't use it anywhere"
18:54:49 mriedem i also have to do cludgy shit like this https://review.openstack.org/#/c/603930/9/nova/compute/manager.py@5189
18:54:52 mriedem if not using an instance_claim
18:54:54 dansmith so, one question I had but was saving was.. are you going to have a resize operation end up going through SHELVED_OFFLOADED from the user's perspective?
18:55:16 mriedem the terminal state is VERIFY_RESIZE for the user
18:55:58 mriedem i have a TODO related to that https://review.openstack.org/#/c/603930/9/nova/compute/manager.py@5024
18:56:47 dansmith because the context would be one place to stash what we're doing to hide that
18:56:51 mriedem i think i need to leave the task_state set there,
18:57:08 mriedem and then on unshelve on the target host, we set the vm_state to RESIZED rather than ACTIVE: https://review.openstack.org/#/c/603930/9/nova/compute/manager.py@5195
18:57:31 mriedem so the functional test is like a normal resize where the caller issues the resize and then polls for VERIFY_RESIZE status
18:58:53 dansmith aight, well, whatever.. point being, it seems bad to create more places where we don't have that set.. more places where if we needed to know which kind of migration we're doing we have to look at the list an guess
18:58:55 dansmith for example,
18:59:18 dansmith we have this long standing bug where we don't properly consider live migrations with pinned cpus as needing to match 1:1 for the destination host,
18:59:23 dansmith which is solvable in other ways,
18:59:43 dansmith but one downstream hack tries to find the migration, determine if it's a live one, so it can make better choices
19:00:09 dansmith which is icky for other reasons, but.. finding out what kind of migration is currently going on and what those details are seems like a good thing to me
19:00:45 dansmith so, not a real helpful opinion, but....there it is
19:01:17 mriedem i can do it either way, it's mostly just a question of how gorby we want the RT resize_claim flow to become when we're unshelving during a cross-cell resize....since as noted we have to do some things manually if we're not doing an instance_claim
19:01:23 mriedem *gorpy
19:01:34 dansmith do we not have any places where we might need to look at a late-stage migration and know if it was cross-cell before we allow or disallow something?
19:02:18 dansmith well, fwiw, the migration context setting via instance_claim never made sense to me, and continues to confuse me and others when we try to remember where it gets set (and why not for things like live migration)
19:02:34 mriedem it's not set in instance_claim, it's in resize_claim
19:02:43 dansmith you know what I mean gdi :)
19:02:46 dansmith in the claim process
19:03:27 mriedem i'm not aware of late stage thingies that rely on the migration_context, maybe there are in the actual finish_resize/finish_revert_resize/confirm_resize flows...which i'm not using
19:03:49 dansmith I'm saying things we'd need to handle in this process, not existing ones
19:03:54 mriedem when we revert/confirm the API looks up the migration directly, not via the migration_context: https://review.openstack.org/#/c/603930/9/nova/compute/api.py@3244
19:04:17 dansmith yeah, which is kinda silly
19:04:31 mriedem which, i guess ^ works because we have the finished status and then it goes to 'completed' or something in the compute
19:04:33 dansmith that would be much nicer as "get the instance and get the current migration" instead of "sort and assume the last one is legit"
19:04:35 mriedem which sound the same to me

Earlier   Later