| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-12-02 | |||
| 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 | |
| 11:52:33 | gibi | the current migration persists the data after the data migration | |
| 11:52:39 | gibi | but only in instance_extra | |
| 11:52:48 | sean-k-mooney | well it only gets saved indretly via eithe a request_spec.save or instance.save | |
| 11:52:58 | gibi | nope | |
| 11:53:17 | sean-k-mooney | wehre do we save the toplogy diretly | |
| 11:53:25 | gibi | sec... | |
| 11:53:52 | gibi | https://github.com/openstack/nova/blob/7670303aabe16d1d7c25e411d7bd413aee7fdcf3/nova/objects/instance_numa.py#L203 | |
| 11:53:55 | gibi | here | |
| 11:54:16 | sean-k-mooney | right so that is in the data migration code | |
| 11:54:27 | gibi | so when an instance is loaded that needed a migration the instance_extra is changed in the DB during the load | |
| 11:54:28 | sean-k-mooney | but normally we do not do that | |
| 11:54:43 | sean-k-mooney | ya only for old instances | |
| 11:54:51 | sean-k-mooney | we can do the same in the request spec | |
| 11:55:08 | sean-k-mooney | wehn its loaded if it has an old instance object it can also just save it | |
| 11:55:53 | gibi | yes, that is what I'm doing but that is ugly | |
| 11:56:10 | gibi | not generic enough to handle the case when object will be persisted in a 3rd | |
| 11:56:10 | gibi | i) ugly ii) does | |
| 11:56:13 | gibi | table iii) create a bad precedence for future OVO data migrations | |
| 11:56:31 | sean-k-mooney | the https://github.com/openstack/nova/blob/7670303aabe16d1d7c25e411d7bd413aee7fdcf3/nova/objects/request_spec.py#L245-L262 | |
| 11:56:36 | gibi | I will do it as I have to provide a fix today downstream that is backportable, but I hate i | |
| 11:56:59 | sean-k-mooney | gibi: well you should not be updating multiple tables really | |
| 11:57:03 | gibi | sean-k-mooney: those are only called if the request spec is created from an instance object | |
| 11:57:18 | sean-k-mooney | you are either udpateing the instance copy or the request spec copy | |
| 11:57:25 | sean-k-mooney | they are not ment to have the same data in them | |
| 11:57:30 | gibi | yes, but from the OVO perspective it does not know whihc | |
| 11:58:07 | gibi | so I cannot have a generic _obj_from_primitive call in InstanceNUMATopology that does the migration as that call has no info where to persist the result | |
| 11:58:09 | sean-k-mooney | the instance request spec object wont | |
| 11:58:17 | sean-k-mooney | but the request spec will know it the request spec | |
| 11:58:39 | sean-k-mooney | you shoudl be fixint this in the request psec object not the InstanceNUMATopology | |
| 11:59:00 | gibi | so now the RequestSpec object has to know that there is an InstanceNUMATopology data migration that needs to be run, that is baaad encapsulation | |
| 11:59:17 | sean-k-mooney | not really | |
| 11:59:36 | sean-k-mooney | the instance numa toplogy object is a child object of the request spec | |
| 11:59:36 | gibi | the data format change should be encapsulated in the InstanceNUMATopology in my eyes | |
| 12:00:09 | sean-k-mooney | gibi: well it can be encaplusted there but there | |
| 12:00:47 | sean-k-mooney | but the request spec shoudl call InstanceNUMATopology to parse the data and also to gereate teh updated serialised version | |
| 12:01:06 | sean-k-mooney | then the the request spec shoudl save that as part of iteslf | |
| 12:01:25 | gibi | but that is bad encapsulation | |
| 12:01:45 | gibi | the data migration should be automatic and opaque from the client of the object | |
| 12:01:59 | sean-k-mooney | gibi: how its just create the InstanceNUMATopology form a json blob and then saving it as a json blob | |
| 12:03:07 | sean-k-mooney | gibi: it woudl be automatic if we bumpt the RequestSpec ovo version when the ovo version of one of its contained objects is increased | |
| 12:03:11 | sean-k-mooney | but we dont | |
| 12:03:31 | sean-k-mooney | so the request spec jsut need to detach that the contained ovo is an older veriion then the current one | |
| 12:04:06 | sean-k-mooney | the constoct the new one using from_primitve and then call to_primate to get the updated version and save it back | |
| 12:04:25 | sean-k-mooney | gibi: we shoudl be doing the data migration in the from_primitive function | |
| 12:04:30 | sean-k-mooney | in InstanceNUMATopology | |
| 12:04:46 | gibi | sean-k-mooney: but then you don't know from the client side if re-presisting is needed or not | |
| 12:04:51 | sean-k-mooney | that will make it transparent to the client and keep the encalupation | |
| 12:05:33 | sean-k-mooney | gibi: well you would if the verion changed but i guess that is a catch 22 | |
| 12:05:57 | sean-k-mooney | we might need a way to ask the InstanceNUMATopology ovo if it need migration | |
| 12:06:17 | sean-k-mooney | by passing it the primiave object form the db and having it return true/false | |
| 12:07:36 | stephenfin | gibi: I think you've figured this out already but no, no blocking DB migration yet. We could add one but it hasn't been done | |
| 12:07:52 | stephenfin | reading back through logs. That looks like a hairy bug | |
| 12:08:08 | gibi | stephenfin: thanks. I will not have time to add the blocking migration now, but maybe at some point in the future | |
| 12:11:18 | sean-k-mooney | is this only needed wehn we have mixed cpus | |
| 12:11:23 | gibi | nope | |
| 12:11:36 | gibi | what you need is a pre victoria pinned instance migrated post Victoria | |
| 12:11:38 | sean-k-mooney | ok so its form the PCPU in placment changes | |
| 12:12:15 | opendevreview | Balazs Gibizer proposed openstack/nova master: Migrate RequestSpec.numa_topology to use pcpuset https://review.opendev.org/c/openstack/nova/+/820153 | |
| 12:12:37 | gibi | sean-k-mooney, bauzas, stephenfin: ^^ that is my first stab to fix this | |
| 12:12:44 | sean-k-mooney | gibi: as a quick hack you can just update the numa toplogy object in the filter | |