| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-02 | |||
| 10:14:53 | bauzas | one way to address this would be to gather the instancenumatopology object from the cell DB at the api level before we hydrate the requestspec | |
| 10:14:55 | gibi | I'm not even sure that for a single instnace the InstanceNUMATopology in the instance and in the request spec are the saem | |
| 10:14:58 | gibi | same | |
| 10:15:21 | gibi | i.e. instance multicreate reuses a single request spec object afaik | |
| 10:15:34 | bauzas | this is fine | |
| 10:15:43 | bauzas | this is just for scheduling decisions | |
| 10:15:48 | bauzas | we don't need to persist this | |
| 10:16:19 | bauzas | that's why we always said we were fine with one single reqspec per boot call in case of multicreate | |
| 10:16:34 | bauzas | and in case of a move op, getting the generic reqspec is ok | |
| 10:16:36 | gibi | then I don't get it | |
| 10:16:56 | gibi | the failure happens during the scheduling of a migration of a pre Victoria instance | |
| 10:17:10 | gibi | in the NumaTopology filter | |
| 10:17:42 | bauzas | gibi: ok, so at the API, we need to gather the instance numa topology from the cell DB corresponding to where the instance is stored | |
| 10:18:08 | bauzas | and we will then hydrate the reqspec with the proper values of the instance | |
| 10:18:41 | gibi | no no, I get that. what I don't get is why the NumaTopology filter depends on this value | |
| 10:19:18 | gibi | what does the request_spec.numa_topology describes? | |
| 10:19:22 | gibi | is the what was requested? | |
| 10:19:46 | gibi | and the instance_extra.numa_topology represent what was actually selected | |
| 10:19:58 | bauzas | excellent question | |
| 10:20:09 | gibi | but then in multicreate there is one request but more than one actual selection | |
| 10:20:39 | gibi | and those selections can be have different values, different pcpu ids | |
| 10:21:30 | gibi | anyhow I have to go back hacking on a solution to fix an urgent downstream issue. The fix needs to be backportable so I will do the ugly code path duplication | |
| 10:21:47 | gibi | later on we can think about a proper fix that reimagine the ovo usage | |
| 10:22:13 | bauzas | good luck | |
| 10:22:18 | gibi | thanks | |
| 10:22:22 | gibi | you will see the result :D | |
| 10:48:03 | gibi | bauzas: how could this ever work? https://github.com/openstack/nova/blob/7670303aabe16d1d7c25e411d7bd413aee7fdcf3/nova/objects/request_spec.py#L727-L730 | |
| 10:48:16 | gibi | as far as I see db_spec is a dict | |
| 10:48:29 | gibi | db_spec.save() is a type error | |
| 10:52:51 | bauzas | gibi: no this isn't a dict | |
| 10:53:12 | gibi | is something connected to a db model? | |
| 10:53:42 | bauzas | gibi: IIRC it returns the class object | |
| 10:54:26 | bauzas | oh wait, sec | |
| 10:55:08 | bauzas | it returns some SQLA object | |
| 10:56:03 | gibi | OK I think I see now, thanks | |
| 10:57:07 | gibi | the unit test simulates the db in a wrong way :/ | |
| 10:58:10 | bauzas | https://www.commitstrip.com/wp-content/uploads/2016/01/Strip-Voyage-dans-le-temps-650-finalenglish-2.jpg | |
| 10:58:50 | bauzas | gibi: I guess we need to pdb it | |
| 10:59:11 | bauzas | my object-persistency-fu is a bit rusty | |
| 10:59:24 | gibi | lol | |
| 10:59:44 | gibi | so the test mocks in a dict instaed of a proper object from the db, hence my problem | |
| 11:00:48 | gibi | moving on | |
| 11:14:09 | gibi | stephenfin: do you remember if we have a blocking db migration for the cpuset -> pcpuset data migration? | |
| 11:29:43 | sean-k-mooney | yay another libosinfo bug that can break live migration this time. | |
| 11:29:49 | sean-k-mooney | gibi: i did not think we needed one | |
| 11:30:33 | gibi | sean-k-mooney: if want to remove the data migration code from InstanceNUMATopology ever then we need a blocking migration to see that all the objects is loaded at least once | |
| 11:31:39 | sean-k-mooney | gibi right but i tought we were not goign to drop that | |
| 11:32:17 | sean-k-mooney | i mean we can but we do it on load to avoid a data migration | |
| 11:32:38 | sean-k-mooney | gibi: back to your instance numa toploty question | |
| 11:32:49 | sean-k-mooney | the one in the request spec is not the same as the one in the instnace | |
| 11:33:05 | sean-k-mooney | the request spec version will not have actul cpus ectra assigned | |
| 11:33:14 | sean-k-mooney | it just has the constriants | |
| 11:33:21 | sean-k-mooney | e.g. 2 numandoes with pinned cpus | |
| 11:33:32 | sean-k-mooney | the one in the instance actully will be fully populated | |
| 11:33:46 | sean-k-mooney | once it has landed on the compute node | |
| 11:33:59 | sean-k-mooney | and been assinged cores and save back to the db | |
| 11:34:58 | gibi | sean-k-mooney: yeah, so we cannot use the data in instance_extra.numa_topology to populate request_spec.numa_topology due to multicreate | |
| 11:35:15 | gibi | we need to independently handle request_spec.numa_topology and migrate it | |
| 11:35:37 | sean-k-mooney | well we shoudl not be populating the request spec version and persiting it | |
| 11:38:01 | sean-k-mooney | gibi: we clone the request spec to avoid it modifying it https://github.com/openstack/nova/blob/7670303aabe16d1d7c25e411d7bd413aee7fdcf3/nova/scheduler/filters/numa_topology_filter.py#L64-L71 | |
| 11:38:18 | sean-k-mooney | so that multi create works | |
| 11:38:49 | sean-k-mooney | stephen fixed that in https://github.com/openstack/nova/commit/bff2030ecea8a1d21e03c61a7ece02f40dc25c5d | |
| 11:39:12 | sean-k-mooney | https://bugs.launchpad.net/nova/+bug/1655979 | |
| 11:39:37 | gibi | sean-k-mooney: so there will be a request spec for each multicreated instance during scheduling but only the first request spec is persisted the rest is just temporary | |
| 11:39:57 | gibi | or we call create() on the clones as well? | |
| 11:40:31 | gibi | ohh | |
| 11:40:41 | sean-k-mooney | no only one will be persited | |
| 11:40:53 | sean-k-mooney | we make a copy and use that in the filter | |
| 11:40:55 | gibi | and that one has numa_topology ? | |
| 11:41:23 | sean-k-mooney | the initial request spec has a partly populated numa_toplogy object | |
| 11:41:28 | sean-k-mooney | which is intentional | |
| 11:42:27 | sean-k-mooney | its create by numa_get_constraits https://github.com/openstack/nova/blob/8d9785b965657d42f20e1ad7234f570077a387d7/nova/virt/hardware.py#L1858 | |
| 11:42:45 | sean-k-mooney | in the api | |
| 11:43:11 | sean-k-mooney | https://github.com/openstack/nova/blob/ff4b396abff80dea5a54dfe830a7db3a97a7360c/nova/compute/api.py#L1068 | |
| 11:43:19 | sean-k-mooney | as part of _validate_and_build_base_options | |
| 11:43:27 | sean-k-mooney | that is what is stored in the request_spec | |
| 11:44:45 | gibi | so that create numa cell objects with cpuset values | |
| 11:45:02 | sean-k-mooney | no the cpuset values will not be populated | |
| 11:45:13 | gibi | https://github.com/openstack/nova/blob/8d9785b965657d42f20e1ad7234f570077a387d7/nova/virt/hardware.py#L1553 | |
| 11:45:48 | gibi | https://github.com/openstack/nova/blob/8d9785b965657d42f20e1ad7234f570077a387d7/nova/virt/hardware.py#L2041 | |
| 11:45:55 | gibi | based on this they will | |
| 11:46:10 | sean-k-mooney | well ok they will https://github.com/openstack/nova/blob/a027b45e46fb3a63166b8b86ef7f99b0b04bcec8/nova/virt/hardware.py#L1552-L1553 | |
| 11:46:51 | sean-k-mooney | from https://github.com/openstack/nova/blob/a027b45e46fb3a63166b8b86ef7f99b0b04bcec8/nova/virt/hardware.py#L2040-L2051 | |
| 11:46:56 | gibi | yepp | |
| 11:47:14 | sean-k-mooney | they wont be mapped to host cores at this point | |
| 11:47:19 | gibi | true | |
| 11:47:38 | sean-k-mooney | but we will know how many floatign or pinned cores are required | |
| 11:48:41 | sean-k-mooney | gibi: what is your current issue by the way | |
| 11:48:48 | sean-k-mooney | i only read back part of the irc log | |
| 11:48:55 | gibi | OK. So i) we need to do the cpuset -> pcpuset data migration in request_spec.numa_topology that was missed originally in Victoria. And we cannot avoid that by populating request_spec.numa_topology from instance_extra.numa_topology | |
| 11:49:29 | gibi | sean-k-mooney: the cpuset -> pcpuset data migration was only done in instance_extra.numa_topology and not in request_spec.numa_topology | |
| 11:49:51 | gibi | so when a pre Victoria pinned instance is migrated after upgrade to Victoria the scheduler blows | |
| 11:50:03 | gibi | as it triest to read out pcpuset but it is not populated | |
| 11:50:33 | gibi | see https://review.opendev.org/c/openstack/nova/+/820121 | |
| 11:51:23 | gibi | so I'm working on a bugfix for it, but it will be ugly as the InstanceNUMATopology object is persisted in two tables but the OVO itself does not know about that fact | |
| 11:51:32 | gibi | so I cannot add a generic solution | |
| 11:51:49 | gibi | I have to keep the upgrading of the two table in separate paths | |
| 11:52:03 | sean-k-mooney | gibi well we can just do the migration in the ovo no? | |
| 11:52:14 | sean-k-mooney | rather then in the db | |
| 11:52:15 | gibi | de ovo does not know which table to update | |