Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-26
14:32:34 dansmith artom: you working on the test failures? I haven't looked to see what they are, but I assume they're relevant since all of them seem to be failing
14:32:40 mriedem https://github.com/openstack/devstack/blob/master/.zuul.yaml#L368
14:32:44 artom dansmith, yeo
14:32:46 artom *yep
14:33:03 artom dansmith, wanted to ask you, we can't fully remove an RPC param until the next major version bump, right?
14:33:38 artom I wanted to remove destroy_disks from the rollback_live_migration call and have the method on the destination do all the deciding, since _live_migration_flags isn't host-dependant, it only cares about migrate_data.
14:33:53 artom But we have to keep compatibility code until RPC 6.0, right?
14:34:21 dansmith artom: fully or otherwise, yeah
14:35:30 artom dansmith, wait, otherwise? So not at all? RPC params are only additive?
14:36:13 dansmith artom: yeah, meaning if you supported the letter and intent of the law in the current version, you can't stop honoring either until the next bump
14:36:16 mriedem what does destroy_disks have to do with numa?
14:36:31 dansmith artom: in that, you can't just accept and ignore some flag that has some meaning
14:37:19 artom mriedem, it's convoluted, but basically, we need to call rollback_live_migration_at_destination if we did a claim and we need to drop it
14:37:35 artom Except now, it's called only the do_cleanup is True (as determined by _live_migration_flags)
14:38:08 mriedem yeah i remmeber that
14:38:22 artom There's a bunch of possibilities around that, but the end result it, everything becomes easier if we just always call rollback_live_migration_at_destination, and let it decide what cleanup needs to be done
14:38:23 mriedem you mean rollback_live_migration_at_destination right?
14:38:36 artom Instead of passing it booleans over RPC
14:38:53 artom Except doing that would technically require removing the destroy_disks param
14:39:47 dansmith artom: don't we pass that flag because there are cases where the other side can't decide what to do?
14:41:02 artom dansmith, might have been the case at one time, but doesn't look like it now. https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6946-L6979 doesn't depend on anything but migrate_data, which both sides have
14:41:11 artom Unless I'm really blind
14:41:26 artom I guess it was done to save an RPC call if we didn't need it?
14:43:55 artom sean-k-mooney, could you weigh in please: https://review.opendev.org/#/c/635669/39/nova/compute/resource_tracker.py@307
14:44:10 dansmith hmm, okay.. I really thought we had serious issues with that logic,
14:44:23 dansmith but perhaps the passing of migrate data to more places made that better or something
14:44:25 dansmith it's been a while
14:44:34 dansmith mriedem probably remembers better
14:44:52 mriedem i would have to dig, _live_migration_cleanup_flags sucks
14:44:57 dansmith yeah
14:45:40 mriedem i had to workaround the same kind of thing where do_cleanup was False but i still needed to do network_api.setup_networks_on_host for the dest in the neutron case with port bindings
14:46:01 mriedem https://review.opendev.org/#/q/I658e0a749e842163ed74f82c975bcaf19f9f7f07
14:47:24 mriedem artom: so you just need the source to make a call to the dest to rollback the claim on failure right? regardless of shared storage or not
14:47:32 artom mriedem, yep
14:47:45 artom I could always just add a new RPC method
14:47:48 mriedem dansmith probably won't like what i'm about to suggest, but it might be better to just write a new rpc method,
14:47:56 artom Seems weird to do that since we already have a thing called "rollback"
14:47:57 mriedem rather than munge it into this steaming pile o shit
14:48:08 artom You don't like wobbly shit castles?
14:48:26 artom If dansmith's onboard I'm all for that
14:48:48 mriedem we'd only make this new call if we know, from the source, that there would be something to cleanup, right? meaning the instance has numa claims in the migrate_data object or something?
14:48:49 dansmith mriedem: I dunno why you'd say that... making another call that does what we want instead of calling this one and expecting very specific behavior is better, IMHO
14:49:06 mriedem dansmith: b/c of our prep_resize hullabaloo a couple of weeks back
14:49:35 artom 'tis settled.
14:49:56 dansmith mriedem: yeah, I mean, okay fair point, but that seemed a little more knife edge single-case to me
15:03:01 sean-k-mooney artom: oh hi yes
15:03:03 mriedem if i re-use the existing prep_resize, i will either have to pass it a new param to control the logic or change it to check if the migration.cross_cell_move flag is set and True (which won't work for old computes, but that could be filtered out in the api or conductor based on compute service version), but either way i'd need to change it to say (1) don't reschedule and (2) don't rpc cast to resize_instance on the source
15:03:16 sean-k-mooney had neutron open for some reason ill take a look
15:09:20 dansmith artom: the new call would be "unclaim $this on the dest" yeah?
15:09:45 sean-k-mooney we used the migration_data for a few reason on of which was move claims were only used for cold migration and we did not want to have to depend on the for sriov migration.
15:09:55 artom dansmith, yep
15:10:11 sean-k-mooney we already had the old vif bindings for the souce and the new vif bindigns for the dest in the migration data
15:10:31 dansmith artom: and what if the source rebooted because of the failure and never calls that?
15:10:39 sean-k-mooney so it was simple to extend that to also have the pci info
15:10:48 dansmith artom: I don't think I was clear about where you're persisting anything from the claim you're doing, other than in memory
15:11:05 sean-k-mooney the only thn we neede was the pci adress which is stored in the vifs port profile
15:11:24 artom dansmith, claims aren't persisted anywhere. The instance has a migration context though
15:11:46 artom dansmith, the update resources periodic would clean up if we don't drop the claim "manually", I believe
15:12:10 dansmith artom: right, the claims themselves aren't, but
15:12:18 dr_gogeta86 hi
15:12:22 dansmith you mean the periodic would find that the migration wasn't associated with us anymore and clean up the in-memory state on the dest
15:12:57 artom dansmith, wait, what do you mean by in-memory state?
15:13:16 artom dansmith, it would update the available resources in the database...
15:13:38 dansmith artom: that's my point, I'm not sure where we're accouting for these resources in the database
15:13:51 sean-k-mooney artom: in the host_cell object in the numa toplogy blob in the compute node
15:14:03 artom dansmith, ^^ there you go :)
15:14:24 dansmith where are we updating that?
15:14:56 artom dansmith, lemme dig
15:15:03 dansmith because I don't see it
15:15:05 artom Knowing sean-k-mooney he might pull it out in seconds
15:15:29 sean-k-mooney dansmith: when we claim the cpus and hugepages in pre live migereation at dest we shoudl update the db
15:15:50 sean-k-mooney that is where we claim the pci deivces for sriov live migration
15:15:52 dansmith sean-k-mooney: yeah, stop using words like "should" and handwaving with "the database".. because that doesn't help :)
15:16:03 dansmith sure, pci devices are claimed with the manager, I get tat
15:16:31 dansmith I don't think we ever claimed the basic resources in the db before we removed all that stuff
15:16:38 dansmith so I'm not sure where this is happening
15:16:51 dansmith mriedem: do you know?
15:17:07 artom dansmith, we don't care about the basic resources anymore, do we?
15:17:11 artom They're all in placement
15:17:15 dansmith artom: no, that's my point exactly
15:17:17 artom I was talking about specific CPUs and stuff
15:17:25 sean-k-mooney well i know that is where we do it for sriov. i know artom is calling the hardware.py module to geenreate the dest numa toplogy and i belive he is claimign the resouces at that point. ill check his patches
15:17:35 artom dansmith, ok, then I didn't understand your point
15:17:51 dansmith sean-k-mooney: again, that's too vague
15:18:21 dansmith artom: I'm just comparing what you're doing here to what we *used* to do with the basic resources, which is effectively what you're "claiming" that you're doing here
15:18:23 mriedem dansmith: well, sort of - we'd update the usage values on the ComputeNode
15:18:37 dansmith mriedem: usage for ram and cpu etc right?
15:18:42 mriedem and disk yeah
15:18:50 artom dansmith, I'm trying to find the specific call that mriedem'd talking about
15:19:02 artom There's a lot of layers, so it's taking a while
15:19:02 mriedem https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1059
15:19:04 dansmith mriedem: did we use that for actual scheduling though? I didn't think we did, but even still, I'm not sure what usage info we have for a complex numa
15:19:25 mriedem pretty sure the core/disk/ram filters looked at the _used values
15:19:55 artom dansmith, so then I think https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1123 would be the NUMA stuff
15:20:02 mriedem https://github.com/openstack/nova/blob/stable/stein/nova/scheduler/filters/core_filter.py#L65
15:20:03 artom Though I haven't traced how that's called, exactly
15:20:11 dansmith mriedem: okay I didn't think they did, but it's been a long time
15:20:25 dansmith mriedem: okay, yeah, host state is coming back to me a bit
15:20:36 mriedem right, HostState is a wrapper over ComputeNode
15:20:43 dansmith artom: that's a blank line

Earlier   Later