| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-26 | |||
| 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 | |
| 15:20:51 | mriedem | i recently found out one of the weighers looks at one of these values as well | |
| 15:20:56 | artom | dansmith, dammit, I was basing it on my local code | |
| 15:20:57 | artom | sec | |
| 15:21:15 | mriedem | ah yes https://github.com/openstack/nova/blob/stable/stein/nova/scheduler/weights/cpu.py#L43 | |
| 15:21:16 | dansmith | mriedem: okay I've dumped all of that out of my head as no longer necessary, thanks | |
| 15:21:34 | dansmith | okay, so _update_usage() is writing a new numa topo for the host | |
| 15:22:04 | artom | dansmith, https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1095 | |
| 15:22:24 | sean-k-mooney | yes the werigher do look at the numa info amoung other things | |
| 15:23:05 | dansmith | artom: okay _get_usage_dict() is what I'm looking for I think | |
| 15:23:39 | dansmith | it looks like we write that compute node record a bunch of times? | |
| 15:24:00 | dansmith | because we call it from update-from-migration, which generates usage and saves it, and would do that again for other migrations and other instances | |
| 15:24:09 | artom | Yeah, I can't imagine we were too efficient with that | |
| 15:24:26 | artom | That code is a whole bunch of _update_blah calling each other | |
| 15:24:44 | artom | It works though | |
| 15:24:59 | sean-k-mooney | so artom is afctully cretein the livemigration claim here https://review.opendev.org/#/c/634606/54/nova/compute/manager.py@6458 in heck_can_live_migrate_destination | |
| 15:25:14 | sean-k-mooney | *check_can_live_migrate_destination | |
| 15:25:31 | artom | At the end of train, Windriver found that the update resources periodic task would remove the new instance's usage because IIRC instance.numa_topology wasn't being updated | |
| 15:25:44 | mriedem | end of train? | |
| 15:25:48 | mriedem | rocky? | |
| 15:25:50 | artom | Err, stein | |
| 15:25:59 | artom | I always mix those up | |
| 15:26:03 | artom | Last cycle | |
| 15:26:38 | dansmith | artom: so to summarize, this new call is a short circuit to the dest to remove the migration usage from the compute node record's topology, which would also happen if the periodic ran and noticed the migration was no longer relevant to it | |
| 15:27:02 | artom | dansmith, yep | |
| 15:27:33 | dansmith | I think part of my problem is, I've mixed up the new-world placement arrangement, with the counting quotas stuff, and then the few remaining things that still use this system | |
| 15:27:44 | dansmith | heaven forbid we account for numbers in the same place | |
| 15:28:27 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Clarify --migration-type migration value as cold migration https://review.opendev.org/678593 | |
| 15:28:37 | sean-k-mooney | well this is really still used to account for assignment of indiviula resouce rather then count the number of resouce avialble | |
| 15:29:00 | artom | dansmith, what sean-k-mooney said. We need to track specific resources like CPU cores, which placement will never do, so this will never fully go away | |
| 15:29:02 | dansmith | in the pci and numa case you mean | |
| 15:29:03 | mriedem | what is "this"? | |
| 15:29:04 | dansmith | sean-k-mooney: ^ | |
| 15:29:18 | artom | this == the resource tracker, and claims | |
| 15:29:22 | sean-k-mooney | no we need to do it for pinned cpus too | |
| 15:29:25 | mriedem | BBBZZZTTT | |
| 15:29:35 | sean-k-mooney | hugepages can be totally done in placmenet however | |
| 15:29:37 | artom | We need to say CPU 3 is used, not just "we have 1 CPU used out of 4" | |
| 15:29:46 | sean-k-mooney | we just need to do the work to move it | |
| 15:29:50 | mriedem | before removing the claims for them, the RT also "tracked" vcpu/ram/disk, just not at some individual device level | |
| 15:29:55 | mriedem | so the RT did both before train | |
| 15:29:59 | artom | mriedem, yep | |
| 15:30:04 | mriedem | hence me and dansmith asking "what is this?" | |
| 15:32:08 | sean-k-mooney | artom: there is a part that im not fully following in your code. i need to do a full review again but where are you calling into the hardware module to calulate the new numa toplogy/cpus/hugepages | |
| 15:32:25 | artom | sean-k-mooney, not directly, it's hidden in the claim | |
| 15:32:43 | sean-k-mooney | that code definetly exsited in the stien version but i have not traced it in the new version | |