| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-27 | |||
| 15:24:17 | sean-k-mooney | https://review.opendev.org/#/c/748453/1 | |
| 15:25:26 | gibi | sean-k-mooney: do you mean we pass the wrong topology during live migration? | |
| 15:25:41 | sean-k-mooney | it depend on where that is being called | |
| 15:25:56 | artom | gibi, I think sean-k-mooney means adding PCI devices to the _live_migration_claim()... | |
| 15:26:08 | sean-k-mooney | artom: well i want to delete that | |
| 15:26:24 | artom | sean-k-mooney, delete... the _live_migration_claim()? | |
| 15:26:25 | sean-k-mooney | but currently its done in check_can_live_migrate_destination | |
| 15:26:35 | sean-k-mooney | artom: i want to remove claims eventually | |
| 15:26:43 | artom | In favor of what? | |
| 15:26:53 | artom | They're like the basis of all of NUMA live migraiton | |
| 15:27:01 | sean-k-mooney | yep i know | |
| 15:27:09 | artom | And I mean... they work. | |
| 15:27:19 | sean-k-mooney | in favor of doing it the way we do for sriov live migration | |
| 15:27:29 | artom | ... why? | |
| 15:27:30 | sean-k-mooney | anyway different topic for a different day | |
| 15:27:36 | gibi | :) | |
| 15:27:46 | sean-k-mooney | port_id_to_pci = self._claim_pci_for_instance_vifs(ctxt, instance) | |
| 15:27:58 | artom | Like yeah, they're hard to grok initially, but they do a bunch of useful things | |
| 15:28:01 | sean-k-mooney | we do that in check_can_live_migrate_destination | |
| 15:28:04 | artom | And they're battle-tested | |
| 15:29:13 | sean-k-mooney | im not sure which numa toplogy is stored in the instance at that point | |
| 15:29:14 | artom | I mean not really for another day, because gibi's patch has to chose 1 or the other... | |
| 15:29:19 | sean-k-mooney | is it the souce numa toplogy or the dest | |
| 15:29:33 | artom | sean-k-mooney, the source, until we apply_move_claim() | |
| 15:29:50 | sean-k-mooney | ok so then that is the wrong numa toplogy for 2 reasons | |
| 15:29:57 | sean-k-mooney | first the souce and dest can differ | |
| 15:30:04 | gibi | I have access to the MoveClaim but that also seems to returning the numa topology stored on the instnace | |
| 15:30:12 | sean-k-mooney | and second if the dest numa toplogy has not been caulated with the numa reuqirement it also wrong | |
| 15:30:43 | sean-k-mooney | we shoudl be geting the toplogy info form the migate_data object right | |
| 15:30:59 | gibi | sean-k-mooney: I did not found it there | |
| 15:31:00 | artom | sean-k-mooney, the whole point of using claims was to calculate the dest numa topology, and store the old and new until we apply the claim... | |
| 15:31:32 | sean-k-mooney | artom: right but sriov migration merged a few months before any for the numa migration stuff did | |
| 15:31:34 | gibi | artom let me know where is the dest numa topology stored and I will modify my patch | |
| 15:31:47 | sean-k-mooney | and it was expictly not using move claims | |
| 15:32:09 | artom | sean-k-mooney, I know, we discussed that during review | |
| 15:32:43 | sean-k-mooney | yes i wanted to move to a case where we claimd resouce in the db and passed the info via migrate data and start using that of all move operations | |
| 15:32:59 | artom | That's what claims do :) | |
| 15:33:04 | sean-k-mooney | badly | |
| 15:33:12 | artom | Why? | |
| 15:33:28 | sean-k-mooney | for one they dont actully store them in the db | |
| 15:33:49 | sean-k-mooney | the are in memroy we are not claiming indivutal resouce on the numa toplogy blob for example | |
| 15:33:52 | artom | Store what? The resource usage? Yes they do, the update the resource tracker, and the migration is consuming the resources | |
| 15:33:54 | sean-k-mooney | the host one | |
| 15:34:25 | sean-k-mooney | claims basically are boolean this is free or it might be used | |
| 15:34:35 | sean-k-mooney | with pci devces we have a tristate | |
| 15:34:37 | artom | Yeah, the eventually call down to rt._update() or w/e it's called | |
| 15:35:07 | sean-k-mooney | available, claimed for an instnace but not used yet and allocated | |
| 15:35:17 | sean-k-mooney | that is stored in teh db drictly in a table not in memory | |
| 15:35:40 | artom | sean-k-mooney, so can the resource tracker already call down to the pci tracker to udpate those? | |
| 15:35:41 | sean-k-mooney | which means if we restart the compute agent that info is not actully lost | |
| 15:35:52 | sean-k-mooney | yes | |
| 15:36:33 | artom | sean-k-mooney, so what's wrong with using that exisiting plumbing? | |
| 15:36:45 | sean-k-mooney | but thats kind of missign the point that i dont really think how we do claimis is a good thing. i know othere wanted to move to a model weher we rely more on plament allocation and directl claims or resouce that are not tempory in memory | |
| 15:36:57 | artom | sean-k-mooney, Placement, sure | |
| 15:37:11 | artom | But let's be honest, NUMA in placement is never happening :P | |
| 15:37:24 | sean-k-mooney | we basically have two source of info here the live migration claim and the migrate_data | |
| 15:37:40 | artom | sean-k-mooney, the migrate data is just the info for the source to update the XML | |
| 15:37:43 | sean-k-mooney | well actully we might eventually have to make that deciesion | |
| 15:37:51 | artom | The claim is for resources consumed | |
| 15:38:05 | sean-k-mooney | there are feature that have been waitn 4+year for numa in placment | |
| 15:38:23 | sean-k-mooney | im not really sure we can justify blocking them to much longer but ill try to get it done next cycle | |
| 15:38:38 | sean-k-mooney | after wallaby however we really need to consier if we will ever do int plamcent | |
| 15:39:03 | sean-k-mooney | we did not need placment ot solve the races we have with numa | |
| 15:39:14 | sean-k-mooney | we just need to not use claims the way they are right now | |
| 15:39:25 | sean-k-mooney | that is one of my main issue with them | |
| 15:39:42 | sean-k-mooney | because the calims are done locally on the compute node it cause the numa races | |
| 15:39:46 | artom | sean-k-mooney, plus, we *already have* PCI stuff in the rt's _move_claim() | |
| 15:40:03 | sean-k-mooney | for what | |
| 15:40:13 | artom | Moves :) Cold migrations | |
| 15:40:13 | sean-k-mooney | oh you put in a hack i remmeber | |
| 15:40:20 | sean-k-mooney | ya for cold migation | |
| 15:40:26 | sean-k-mooney | but also the hack for live | |
| 15:40:35 | sean-k-mooney | where we dont set them or something | |
| 15:41:54 | sean-k-mooney | this https://github.com/openstack/nova/blob/f521f4dbace0e35bedd089369da6f6969da5ca32/nova/compute/resource_tracker.py#L303-L314 | |
| 15:42:11 | artom | gibi, so https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L223 is the live migration claim creation method, that creates the migration context with the old and new NUMA topologies by calling down to _move_claim() | |
| 15:42:53 | artom | gibi, and yeah, sean-k-mooney found my TODO about converging the 2 (NUMA and SRIOV live migration) | |
| 15:43:39 | artom | gibi, the live migration claim is created on the destination at https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L7670 | |
| 15:43:40 | sean-k-mooney | right which i objected too at the time and still do unless we remove the migration_data object | |
| 15:43:56 | sean-k-mooney | and and a couple of other things | |
| 15:44:14 | sean-k-mooney | like moving claims for spawn and other vent to the conductor | |
| 15:44:25 | sean-k-mooney | or ideally earlier | |
| 15:44:28 | artom | gibi, and then we eventually save the dest stuff (like numa topology) here: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L8587 | |
| 15:45:13 | sean-k-mooney | artom: if its not clear as far as i am concerned the main reason we have races with numa is due to claims beign done on the compute node. that is what placment and numa in plamcnet was ment to solve | |
| 15:46:24 | sean-k-mooney | or we could have fixed it years ago if claims were dont in the scuderler of conductor | |
| 15:46:39 | sean-k-mooney | gibi: https://github.com/openstack/nova/blob/f521f4dbace0e35bedd089369da6f6969da5ca32/nova/compute/resource_tracker.py#L328 | |
| 15:46:59 | sean-k-mooney | gibi: you need to use the new_numa_topology for the migration context | |
| 15:47:13 | artom | sean-k-mooney, it's not so much *where* they're done, it's that scheduling and claiming happened in 2 steps | |
| 15:47:30 | sean-k-mooney | artom: yes which is where they are done | |
| 15:47:32 | artom | sean-k-mooney, but yeah, claiming in the scheduler would have fixed that, because we could have made schedule + claim atomic | |
| 15:47:42 | sean-k-mooney | yes | |
| 15:47:53 | sean-k-mooney | which is what i argured for befor placment was created | |
| 15:48:06 | artom | Yeah, I could get behind that | |
| 15:48:13 | artom | But it's moot at this point | |
| 15:48:19 | artom | For better or worse | |
| 15:48:33 | mriedem | search is busted in the docs https://docs.openstack.org/nova/latest/search.html?q=cross_az_attach - click on one of those | |
| 15:48:55 | sean-k-mooney | oh it is | |
| 15:48:58 | sean-k-mooney | did we move things | |
| 15:49:11 | gibi | sean-k-mooney, artom: thanks I think I see it now | |
| 15:49:12 | sean-k-mooney | availability-zonesundefined | |
| 15:49:17 | artom | Where did the 'undefined' in that URL come from? | |