| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-03 | |||
| 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 | |
| 14:57:21 | sean-k-mooney | but it was doable without the db level tracking | |
| 14:57:26 | sean-k-mooney | so we did not modify the db | |
| 14:57:48 | artom | You mean for stuff like hard reboot? | |
| 14:57:58 | openstackgerrit | Pavlo Shchelokovskyy proposed openstack/nova master: Raise 409 when removing security group from instance https://review.opendev.org/735572 | |
| 14:58:03 | sean-k-mooney | since we coudl rely on plamcnet for atomic claims of resouces we coudl skip doning that in nova | |
| 14:58:14 | artom | OTOH, it's not like vGPUs have state, so if it gets a different mdev after hard reboot, does it really matter? | |
| 14:58:35 | artom | sean-k-mooney, definitely agree with the general idea | |
| 14:58:36 | sean-k-mooney | if the mdev type does not change no | |
| 14:58:50 | sean-k-mooney | for pmem its much more important | |
| 14:59:03 | sean-k-mooney | which is why we now have https://github.com/openstack/nova/blob/c57d52e1978210fb3260bf8ea442237d548db576/nova/objects/resource.py | |
| 14:59:10 | sean-k-mooney | so that we can do this genericly | |
| 14:59:20 | artom | sean-k-mooney, ah, so for when it matters we *do* persist it in the DB | |
| 14:59:34 | sean-k-mooney | we do now yes | |
| 14:59:49 | sean-k-mooney | and we have a generic way to do it in the future without db modficiations | |
| 15:00:11 | sean-k-mooney | its one of the thing i pushed for durign the pmem stuff which alex_xu and other kindly implemented | |
| 15:01:22 | artom | sean-k-mooney, so yeah, NUMA in placement would be nice, and would solve a whole bunch of problems, but it's a mess to make work, so here we are ^_^ | |
| 15:02:07 | sean-k-mooney | this really doesnt have that much to do with numa in placment although this generic resouce field would allow use to too remove teh numa blobs if we had that | |