Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-02
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 stephenfin sean-k-mooney: when do we ever remove things when we said we would
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: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
13:58:37 gibi so I dropped that direction
13:59:10 sean-k-mooney sorry i normally expect those to be func tests but ya backporting the func test to victoria woudl be non trivail
13:59:26 sean-k-mooney in this specific case i think a unit test is ok
13:59:40 gibi not that what I meant. I mean creating a func test on master that simulates a pre Victoria instance is hard
14:00:15 gibi I would need to dig in to the DB anyhow to backlevel the data to pre-victoria as our object layer will persist the master version
14:00:34 sean-k-mooney yes so you woudl have to boot the vm
14:00:40 sean-k-mooney then update its request spec
14:00:49 sean-k-mooney then migrate it and assert it expodes
14:01:22 sean-k-mooney gibi: i assumed you ment because we willl be missing several fo the helper functions
14:01:32 gibi not that ^^
14:01:40 gibi but the "then update its request spec"
14:01:54 gibi that would be heavy DB digging in a func test
14:03:54 sean-k-mooney gibi: ya you woudl effectly have to directly execute sql
14:04:00 sean-k-mooney which i think is over kill
14:04:17 gibi and in that sql manipulate a highly nested json dict :D
14:04:18 sean-k-mooney and this is all in a json blob in the db too so its not exactly nice to update via sql either
14:04:25 gibi exactly :D
14:04:25 sean-k-mooney exactly
14:04:33 sean-k-mooney jinks :)
14:04:57 sean-k-mooney so ya its understandable that you chose to go the unit test route
14:04:58 gibi I have a downstream env where they can reproduce the issue so I will have real test result hopefully in a day
14:05:45 gibi that env is a complicated one simulating an upgrade from Mitaka to Victoria :D
14:06:02 gibi so when I say pre-Victoria instance that is actually a Mitaka instance :D
14:06:25 sean-k-mooney i suspect the only reason we have not hit this downstream is our last release was based on train
14:06:42 sean-k-mooney and the next one which will be based on wallaby is not releasing until next year
14:06:52 gibi yepp, the bug is introduced in Victoria
14:07:06 sean-k-mooney that is after the initall cpu in placement work
14:07:09 sean-k-mooney that was in train
14:07:19 sean-k-mooney this is part of the mixed cpu feature i think
14:07:55 sean-k-mooney ya part of https://specs.openstack.org/openstack/nova-specs/specs/victoria/implemented/use-pcpu-vcpu-in-one-instance.html

Earlier   Later