Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-02
10:06:57 bauzas hah
10:06:59 gibi it is ugly and non generic
10:07:03 bauzas got it
10:07:26 bauzas well, this should have been done this way in Victoria either way, right?
10:08:15 bauzas problem is, we assume that InstanceNumaTopology object is only persisted by instance_extra db table
10:08:18 bauzas right?N
10:09:00 gibi right
10:09:03 bauzas if so, the object is broken
10:09:13 bauzas I mean the object persistency
10:09:24 gibi the problem is that the InstanceNUMATopology object does not know that it is used in two differnt context
10:09:26 bauzas we need to say we have to persist in two db tables
10:09:45 bauzas gibi: yup, hence my word 'broken'
10:10:03 bauzas in general we have object classes that are backed by a single db table
10:10:13 gibi I can add a generic code to _obj_from_primitives that called in both case and do the data migration genericly, but in that code I cannot decide which table to write the data back
10:10:17 bauzas and we directly map the object fields with the db values
10:10:35 bauzas gibi: agreed with your problem, this is a pain to fix
10:10:45 bauzas the design itself is having flaws
10:11:29 bauzas we somehow need to have ovo objects that know which db table they are related
10:11:47 bauzas but honestly, I somehow feel those necessarly have to be two different objects
10:12:04 bauzas we can nest objects under others
10:12:34 bauzas but given RequestSpecs is at the API DB, we can't just hydrate the value from the cell DB values
10:13:03 bauzas or, the other way to consider that, is that we only use RequestSpecs.InstanceNUMATopology as a non-persisted object
10:13:45 bauzas but we would need to scatter/gather the values from the cell DB before we hydrate such reqspec nested object
10:14:25 bauzas gibi: see ?
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

Earlier   Later