Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-12-02
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 sean-k-mooney exactly
14:04:25 gibi exactly :D
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
14:08:27 sean-k-mooney https://specs.openstack.org/openstack/nova-specs/specs/victoria/implemented/use-pcpu-vcpu-in-one-instance.html#work-items
14:08:35 sean-k-mooney it added the pcpuset filed
14:10:22 sean-k-mooney initally after the pcpu in placment change we just used cpuset since all cores were either pinned or not and we coudl tell that from teh config cpu_policy
14:10:48 gibi yepp that is my understanding too
14:14:10 opendevreview Balazs Gibizer proposed openstack/nova master: Migrate RequestSpec.numa_topology to use pcpuset https://review.opendev.org/c/openstack/nova/+/820153
14:14:18 gibi now with release notes :)
14:17:43 sean-k-mooney :)
14:17:57 sean-k-mooney ill wait for ci to finsih and ill try an re review later
14:19:34 gibi thanks
17:43:40 opendevreview Balazs Gibizer proposed openstack/nova master: [WIP]Stop persisting RequestSpec.numa_topology https://review.opendev.org/c/openstack/nova/+/820215
#openstack-nova - 2021-12-03
00:45:37 opendevreview Ghanshyam proposed openstack/nova-specs master: Allow project admin to list hypervisors https://review.opendev.org/c/openstack/nova-specs/+/793011

Earlier   Later