| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-03 | |||
| 14:21:04 | sean-k-mooney | stephenfin: by edgecase you mean the fact we dont consider the numa toplogy of the guest at all right now | |
| 14:21:14 | artom | Also, having NUMA and SRIOV in the same instance is not an edge condition :) | |
| 14:21:19 | artom | Unless I'm way off base on that bug | |
| 14:21:32 | stephenfin | sean-k-mooney: it's to do with the claiming bit right? | |
| 14:21:38 | sean-k-mooney | yes | |
| 14:21:46 | sean-k-mooney | when we claim we pass None currenlty | |
| 14:21:50 | stephenfin | okay, so I have https://review.opendev.org/#/c/749360/ | |
| 14:22:34 | artom | stephenfin, ah, you're not using NUMA | |
| 14:22:36 | artom | Just SRIOV | |
| 14:22:36 | stephenfin | but the guest NUMA topology appears to be getting correctly recalculated to ensure the instance lands on the same host NUMA node as the PCI device | |
| 14:22:42 | artom | Not in the same test, at least | |
| 14:22:47 | stephenfin | I am in that one | |
| 14:22:51 | stephenfin | I'm using CPU pinning | |
| 14:22:55 | stephenfin | that's NUMA | |
| 14:23:14 | stephenfin | https://review.opendev.org/#/c/749360/1/nova/tests/functional/libvirt/test_pci_sriov_servers.py@363 | |
| 14:24:08 | sean-k-mooney | it could be passing jsut out of luck | |
| 14:24:21 | artom | Interesting, I wonder if there's a specific assertion that's missing | |
| 14:24:32 | sean-k-mooney | you would need to force a case where cpus are only avaiable on one node and the pci device is on another | |
| 14:24:38 | sean-k-mooney | with the bug it would boot | |
| 14:24:46 | stephenfin | sean-k-mooney: Yeah, I think that's what I'm missing | |
| 14:24:56 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: compute: Validate a BDMs disk_bus when provided https://review.opendev.org/744552 | |
| 14:25:09 | stephenfin | But I wasn't able to come up with a clever way to do that since conf is global :-( | |
| 14:25:12 | sean-k-mooney | by the way gibi's fix i dont thnk it a full fix | |
| 14:25:53 | sean-k-mooney | since we are claiming the devices at a different point then we are generating the numa toplogy there is always a race with outher vms unless this is all happening under a common lock | |
| 14:26:00 | sean-k-mooney | and i dont think it is | |
| 14:26:16 | sean-k-mooney | gibi's patch makes the code stricly better | |
| 14:26:40 | sean-k-mooney | but it does not make the numa toplogy generation and pci claims effectly atomic | |
| 14:31:08 | stephenfin | I'm getting the impression, based on those resize/cold migrate resize/confirm bugs, that pretty much everything the resource tracker is doing has to be atomic | |
| 14:31:56 | sean-k-mooney | where we dont have tristate logic like the pci tracker yes definetly | |
| 14:32:35 | sean-k-mooney | but in general yes | |
| 14:32:54 | sean-k-mooney | that is why we have the lock | |
| 14:33:45 | artom | The whole periodic using the migrations and flavors and instances is a mess | |
| 14:35:02 | artom | Kinda makes me wonder if there's a still a point to it... | |
| 14:35:22 | artom | We've gotten a lot better at making migrations correctly track resources, especially live migrations | |
| 14:35:34 | artom | Might make more sense to address the remaining gaps, and then remove it altogether | |
| 14:35:40 | artom | Or make it a nova-manager command, with a lock | |
| 14:35:48 | artom | Kinda like the placement audit thing | |
| 14:36:33 | sean-k-mooney | you mean in placment | |
| 14:36:52 | sean-k-mooney | in theory you should be abel to run nova with that periodic disabled | |
| 14:37:04 | sean-k-mooney | in paratics i expect that to be super broken | |
| 14:37:27 | artom | sean-k-mooney, no, I mean fix the remaining problems with resources not being correctly updated | |
| 14:37:33 | artom | And stop running the periodic | |
| 14:38:00 | sean-k-mooney | yes but that means basicaly persit stuff to the db properly | |
| 14:38:30 | sean-k-mooney | so stop usign move cliams and claims in general in memory and actully claim the stuff in the db atomicly | |
| 14:40:08 | artom | Well claims do update the usage eventually | |
| 14:40:24 | sean-k-mooney | part of the reason we have to rebuild the resouce state form flaovr/instance/migration is because we dont store it anywhere but in memory | |
| 14:40:58 | sean-k-mooney | artom: sure but if you restart the agent then you have to rebuild the state again on start up | |
| 14:41:17 | sean-k-mooney | form the migrations and isntance exctra | |
| 14:41:56 | artom | sean-k-mooney, https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L344 | |
| 14:42:14 | sean-k-mooney | yes that update teh compute nodes table | |
| 14:42:21 | sean-k-mooney | that is not the same thing | |
| 14:42:53 | sean-k-mooney | oh thats in the move claim | |
| 14:43:00 | sean-k-mooney | its not updateign the db | |
| 14:43:11 | sean-k-mooney | its updating the resouce tracker | |
| 14:43:11 | artom | How so? | |
| 14:43:33 | artom | sean-k-mooney, https://github.com/openstack/nova/blob/c57d52e1978210fb3260bf8ea442237d548db576/nova/compute/resource_tracker.py#L1185 | |
| 14:44:01 | sean-k-mooney | the compute node table does not containe all the data in the resouce tracker | |
| 14:44:04 | sean-k-mooney | just a summary | |
| 14:44:14 | sean-k-mooney | it has non of the vgpu mdeves for example | |
| 14:44:34 | artom | Aren't those in placement? | |
| 14:44:38 | sean-k-mooney | no | |
| 14:44:52 | sean-k-mooney | the count of how many we can create is | |
| 14:45:28 | artom | But we have to care about the specific individual ones like we do with PCPUs? | |
| 14:45:28 | sean-k-mooney | not the mdev paths for the vgpu instnace | |
| 14:45:46 | sean-k-mooney | artom: yes and no | |
| 14:46:05 | sean-k-mooney | artom: if you have multiple devices or multipel mdev_types enabled on the host yes | |
| 14:46:15 | sean-k-mooney | artom: we create one RP per pGPU | |
| 14:46:41 | sean-k-mooney | so the allcoation need to be collated to the correct pgpu on the host and the mdev need to be created form that gpu | |
| 14:46:53 | sean-k-mooney | that info is stored only in memory | |
| 14:47:13 | sean-k-mooney | well we also parse the libvirt xmls when the agent restarts too | |
| 14:47:41 | artom | The host capabilities you mean? | |
| 14:48:18 | sean-k-mooney | no what do you mean by host capablities | |
| 14:48:25 | artom | You said XML | |
| 14:48:32 | artom | I'm asking *which* XML :) | |
| 14:48:32 | sean-k-mooney | yes i ment the domain xml | |
| 14:48:46 | artom | Ah, that lists the mdevs that each instance uses | |
| 14:49:01 | sean-k-mooney | when the agent restarts since we dont store the mdev a vm is using in the db we have to look it up for the running vms | |
| 14:49:14 | artom | Hrmpf :( | |
| 14:49:41 | sean-k-mooney | i argued it shoudl be stored in the db either in its own tabel or in the host numa toplogy blob | |
| 14:49:47 | sean-k-mooney | for what its worth | |
| 14:50:09 | sean-k-mooney | its true that in generall the uuid we use does not matter to the vm | |
| 14:50:19 | sean-k-mooney | but there are edgecase where we do care like agent restart | |
| 14:51:43 | sean-k-mooney | artom: for what its wort we have added a new generic resouce filed ot the instance_extra_tables | |
| 14:51:45 | sean-k-mooney | https://github.com/openstack/nova/commit/6a4d6ec786bb35b0bf81d91b867d55f4f8b1e32d | |
| 14:51:57 | sean-k-mooney | that could be used for mdev for vgpus too if we wanted | |
| 14:52:10 | sean-k-mooney | it was created to be generic so we could reuse it | |
| 14:52:34 | sean-k-mooney | its currently only used for pmem | |
| 14:53:24 | sean-k-mooney | the pmem resouce are also not in the comptue nodes table unless they are embeed in say the host numa_toplogy bolb | |
| 14:53:55 | sean-k-mooney | unless i missed something | |
| 14:54:19 | artom | I don't have enough context, but maybe they don't need to be persisted in the DB? | |
| 14:54:43 | artom | Like, what does it affect? For scheduling, we only need to know quantities of mdevs, not individual ones, no? | |
| 14:55:00 | sean-k-mooney | we do but we use placement for the sechduling aspect | |
| 14:55:19 | sean-k-mooney | artom: for vGPUs you mean | |
| 14:55:25 | artom | sean-k-mooney, yeah, for example | |
| 14:55:42 | artom | Building the XML of new instances? Do we need to now the paths of available mdevs? | |
| 14:56:03 | sean-k-mooney | quantity and mdev_type which can be tracked via triats is enough in that specific case | |
| 14:56:25 | sean-k-mooney | artom: we need to create an mdev and put the path in the xml | |
| 14:56:41 | artom | So based on that seems like not tracking them was maybe the correct choice? | |
| 14:56:59 | sean-k-mooney | yes and no | |
| 14:57:11 | sean-k-mooney | it creates a depenency on the domain xmls for running guest | |