Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-02
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
12:26:28 sean-k-mooney we woudl still need to store it in the request spec to not chagne ethe filter api
12:44:29 bauzas sean-k-mooney: that's something we already do
12:44:42 bauzas sean-k-mooney: we either populate from the image and/or the flavor first
12:44:55 bauzas but then for move ops, we get it from somewhere else
12:47:26 sean-k-mooney where? we currently populate it form the instance or when we build the request spec form its componets
12:48:20 sean-k-mooney bauzas: gibi we coudl make it a propery and constuct it form the image and flavor if we needed too
12:48:25 sean-k-mooney and stop persiting it
12:51:49 sean-k-mooney it might slow donw scduling some what as numa_get_constraints is not super cheap but its also not that expensive either
12:52:34 sean-k-mooney its a pure fucntion of its inputs https://github.com/openstack/nova/blob/7670303aabe16d1d7c25e411d7bd413aee7fdcf3/nova/virt/hardware.py#L1858 but we would have to make sure to use the correct falvor on resize
13:48:26 gibi sean-k-mooney: you mean not persist the numa_topology in the RequestSpec at all? Just create it from the image/flavor every time we need it?
13:50:20 sean-k-mooney gibi: yep that is what bauzas is suggesting
13:50:32 sean-k-mooney which we could actully do in the case of the request sepc
13:50:45 bauzas sorry, a bit busy on and off
13:51:00 bauzas but yeah I don't like to persist the same DB table in another DB
13:51:04 sean-k-mooney teh numa constreait fucntion is relitvely cheap in comparison to the actul numa affinity process
13:51:14 sean-k-mooney bauzas: its really two differnt thigns
13:51:26 sean-k-mooney we jsut use the same object
13:51:36 sean-k-mooney one is the numa affinity request object
13:51:56 sean-k-mooney and the other is the final affinty object which stores the assinged cpus and numa nodes
13:52:07 sean-k-mooney we just reused the same object for both
13:52:12 gibi OK. I got it thanks.
13:53:31 sean-k-mooney gibi: if we go with the generate it when needed approch i would prefer not to backport that mainly because i dont want to have to thing do we have that or not when looking at different downstream releases
13:53:54 sean-k-mooney but im not against it for master on
13:53:56 gibi yeah I would not backport that either
13:54:22 gibi Lets see if I can get some time adding that to master
13:54:34 gibi but I will go with the current patch as a backportable thing
13:54:35 bauzas sean-k-mooney: I understand that's two different things
13:54:43 bauzas but using the same object is creating some concerns
13:54:49 gibi we need that to move back til victoria
13:55:11 gibi bauzas: totally agree I think it is worth to remove it from the second table
13:55:20 bauzas ++
13:55:34 sean-k-mooney gibi: i review that and +1'd by the way the only thing i woudl add is a release note but it looks correct to me
13:55:53 sean-k-mooney well actully
13:55:59 sean-k-mooney did you update the func test
13:56:16 sean-k-mooney i closed the review
13:56:48 sean-k-mooney ok you did not
13:57:04 sean-k-mooney so ya release note and then you need to fix the repoduce func test
13:57:18 gibi sure I will add a reno
13:57:24 sean-k-mooney which passed...
13:57:26 gibi the reproduce was fixed but it is not a func test it is a unit
13:57:41 sean-k-mooney oh ok
13:57:45 sean-k-mooney ya it is
13:57:59 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/820153/1/nova/tests/unit/objects/test_request_spec.py
13:58:30 gibi creating a pre victoria instance in the func test is far from trivial

Earlier   Later