Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-02
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 i) ugly ii) does
11:56:10 gibi not generic enough to handle the case when object will be persisted in a 3rd
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 gibi the data format change should be encapsulated in the InstanceNUMATopology in my eyes
11:59:36 sean-k-mooney the instance numa toplogy object is a child object of the request spec
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
12:13:46 sean-k-mooney taht wont actully fix it will it. well i twill work but it wont update the request spec version
12:14:23 gibi yeah that would be an option too If I can assume that only the numa filter depends on this
12:14:27 sean-k-mooney ah you also do it there
12:14:44 gibi I do it now in the RequestSpec loading
12:15:03 sean-k-mooney yep just lookg at it now
12:15:14 sean-k-mooney we use it in the api also
12:15:25 sean-k-mooney wehn validating rebuilds
12:15:42 sean-k-mooney although im not sure we actully depend in the cpu sets directly
12:15:58 sean-k-mooney i think we generate new objects form the old and new image
12:16:05 sean-k-mooney and assert they are the same
12:16:23 stephenfin gibi: that's how I'd have done it also
12:17:38 sean-k-mooney stephenfin: in the filter or gibis patch
12:17:45 stephenfin gibi's patch
12:18:00 sean-k-mooney ya i was assuming we would do it on load
12:18:19 sean-k-mooney to avoid the blocker migration
12:18:23 stephenfin doing it in the filter leaves us having to support that forever. As a temporary fix, sure, but it's not viable long-term
12:20:17 sean-k-mooney gibi im not sure we can assume we can remove this imideatly after yoga
12:20:43 sean-k-mooney we might want to keep it for a few releases in the event that peoppel do an inplace upgrade
12:20:43 stephenfin sean-k-mooney: when do we ever remove things when we said we would
12:20:53 gibi :D
12:20:59 sean-k-mooney lol true
12:21:08 gibi sean-k-mooney: technically if we add a blocking migration then we can remove this in Yoga
12:21:17 gibi probably we wont add that
12:22:00 sean-k-mooney gibi: we could also do it as a nova manage command with a nova status check
12:22:16 gibi yepp
12:22:25 bauzas honestly, my jab is that we shouldn't persist this object in the API DB
12:22:26 gibi if we trust the users to run the upgrade check
12:22:38 sean-k-mooney bauzas: no the curent behavior is correct
12:22:41 bauzas if it's just for asking the scheduler using it
12:22:54 sean-k-mooney bauzas: we need to persit it to persit the numa constraits
12:23:11 sean-k-mooney well technially we can generate it again
12:23:13 bauzas which service is persisting it ?
12:23:19 sean-k-mooney if we have the embeded image and flavor
12:23:36 sean-k-mooney it capture the numa scheduleing requirements
12:23:50 bauzas I need to taxi my daughter now
12:23:54 bauzas but let's discuss this after
12:23:59 sean-k-mooney if we dont persite it we need to regenerate it form the embeded flavor and image
12:25:19 bauzas or from the instance one when moving , nope ?
12:25:36 bauzas anyway, me moving
12:26:10 sean-k-mooney no the instance object is fully populated it might work but its would not work for inital schduling

Earlier   Later