Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-08-05
11:54:32 sean-k-mooney if and only if that RC exist on only one nested RP we can heal it
11:54:41 gibi cyborg is requested via device profile name in the flavor so that can be detected easier, for any CUSTOM_FOO your idea is viable
11:55:11 sean-k-mooney but if the same RC exits on multiple RPS really only the virt dirver would be able to figure out what rp is correct
11:55:48 sean-k-mooney e.g. it would have to use inform form the libvirt domain or similar to try and determin which pGPU the vGPU was allocated form
11:56:23 gibi sean-k-mooney: yes, the ambiguity cannot be resolved in the CLI, we have that thing already implemented (rejected) for the QoS healing
11:56:46 gibi there the VF - PF - port - PF RP relationship cannot be disambiguated in the CLI
11:57:30 sean-k-mooney part of me thinks we should be storing some addtion info in our db to allow this
11:57:40 sean-k-mooney but im not sure what that should be
12:01:26 sean-k-mooney what annoys me about this problem is even if we save a copy of the inital allocation summeries and the placment query that we used to generate it im not conviced that is sufficent to reconstuct the allocations remotely
12:01:46 gibi I stop here now. I will add some blocking and documentation to the CLI about vgpu and cyborg dev profile to prevent damage. then I will continue adding QoS support. other can take up removing the vgpu and cyborg block by implementing support
12:02:09 sean-k-mooney gibi: it kind of feels like the only way to do this would be to have nova-manage call the virt dirver over rpc to have it fix it
12:02:31 sean-k-mooney gibi: ack
12:02:44 gibi yes, the full support most probably would need that
12:03:00 opendevreview Lee Yarwood proposed openstack/nova master: zuul: Mark live migration jobs as non-voting due to bug #1912310 https://review.opendev.org/c/openstack/nova/+/803585
12:03:34 songwenping sean-k-mooney: we have a problem on cyborg that you may know, please give some tips if you have free time.
12:03:47 songwenping https://review.opendev.org/c/openstack/cyborg/+/797403 this patch backport to victoria, but it requires oslo.db==10.0.0 for cybort, because this patch https://review.opendev.org/c/openstack/oslo.db/+/792124 resolved duplicate key error for mysql. how can we fix the cyborg tempest?
12:04:08 songwenping https://review.opendev.org/c/openstack/cyborg/+/797403 this patch backport to victoria, but it requires oslo.db==10.0.0 for cyborg tempest, because this patch https://review.opendev.org/c/openstack/oslo.db/+/792124 resolved duplicate key error for mysql. how can we fix the cyborg tempest?
12:06:56 sean-k-mooney ok so you are gettign duplicate DeviceProfile uuids in this case 977806ca-4e8e-40c2-aa3a-09cef2903336
12:08:07 sean-k-mooney and this is currently how you create your device profiles https://github.com/openstack/cyborg/blob/1052efe93b5e7aa351b1f50cfe80f504dcf48b72/cyborg/db/sqlalchemy/api.py#L499-L517
12:10:51 sean-k-mooney so this is the contraing that is filing
12:10:53 sean-k-mooney pymysql.err.IntegrityError: (1062, "Duplicate entry 'fpga_same_test' for key 'device_profiles.uniq_device_profiles0name'")
12:14:48 sean-k-mooney ok i see
12:14:52 sean-k-mooney https://review.opendev.org/c/openstack/oslo.db/+/792124/6/oslo_db/sqlalchemy/exc_filters.py
12:15:11 sean-k-mooney so thet way that code is ment to work is it parses device_profiles.uniq_device_profiles0name
12:16:00 sean-k-mooney and it should extract the unique constrait by firsts discarding eveything before uniq_ leaving device_profiles0name
12:16:34 sean-k-mooney the it splits that on the 0 to get the table device_profiles and columns in this case name
12:17:10 sean-k-mooney so the unique constrati you were expecting was device_profiles name not the uuid
12:19:12 sean-k-mooney without that fix it was using all colums as unique constriats? i guess at least that the oslo db level
12:19:42 sean-k-mooney oh i see
12:20:11 sean-k-mooney In mysql 8.0.19 , Duplicate key error information is extended to
12:20:13 sean-k-mooney include the table name of the key.Previously, duplicate key error
12:20:15 sean-k-mooney information included only the key value and key name.
12:21:46 bauzas gibi: sorry was out for lunch
12:22:05 sean-k-mooney songwenping: im surpised that affect https://github.com/openstack/cyborg/blob/1052efe93b5e7aa351b1f50cfe80f504dcf48b72/cyborg/db/sqlalchemy/api.py#L511-L516
12:22:15 bauzas gibi: about the VGPU healed allocations, well, we already verify the VGPU RC for the audit command but we don't do this for the heal_allocations
12:22:55 bauzas gibi: also, given we would now use other custom RCs, maybe we could also modify both the audit and heal_allocs commands to have a new attribute for telling which RCs to look at
12:25:15 songwenping sean-k-mooney: yes, the mysql version is update.
12:25:18 sean-k-mooney songwenping: without the oslo.db patch teh colum will be named "device_profiles.uniq_device_profiles0name" with it it will be "name"
12:25:20 sean-k-mooney oh i see
12:25:43 sean-k-mooney ok i get what is happening now so e.columns is a dict
12:26:01 sean-k-mooney if 'name' in e.columns: is checkking if there is a key that exactly match
12:26:36 sean-k-mooney the keey has chaned to now have the table name prefixed
12:26:54 sean-k-mooney so the fix you can do in cyborg is to add an elif
12:27:47 songwenping right, the e.columns isnot ['name'] any more.
12:29:34 sean-k-mooney ya so we just need to make the comparisone a little more robost
12:30:44 songwenping if i make the name and id all conflict, the e.columns is ['id'] and isnot ['id','name']
12:31:32 sean-k-mooney https://paste.opendev.org/show/807904/
12:31:47 sean-k-mooney i think this will work ^
12:32:33 songwenping this is good for now.
12:32:59 sean-k-mooney https://paste.opendev.org/show/807905/
12:33:03 sean-k-mooney or maybe that
12:33:38 sean-k-mooney add an else just in case we have a conflicat that is not on name or uuid although it would be treated as a uuid conflict today
12:34:36 sean-k-mooney songwenping: but ya the other way to do this is to preporcess the columns dict and stip the table prefix
12:35:57 songwenping does other projects have the same problems?
12:36:32 songwenping i see only cyborg distingush the uuid and name conflict.
12:36:41 sean-k-mooney i think we might define unique constraints differently then cyborg does
12:39:20 sean-k-mooney songwenping: so this is the other way to fix it https://paste.opendev.org/show/807908/
12:39:57 sean-k-mooney all that has changed here is instead of using e.colums directly i have generated a new columns dict and then the exsitng if else just uses that
12:39:59 songwenping this is same as the oslo.db does.
12:40:07 sean-k-mooney yep more or less
12:40:32 songwenping but whether it depends on mysql version
12:40:44 sean-k-mooney so on brances that cant use the new version fo oslo db you can backport that in cyborg
12:41:01 sean-k-mooney songwenping: this will work for any mysql verion
12:41:24 sean-k-mooney if you have old mysql it will be a noop as none of the column names will have 0 in them
12:41:35 songwenping ok, this is a good idea, thanks.
12:41:42 sean-k-mooney so columns and e.columns will be the same
12:42:03 songwenping right
12:42:26 sean-k-mooney that is proably the minimal change let me see quickly what is different between how nova defines unique constraints an cyborg
12:44:40 songwenping nova doesnot distingush the conflict types
12:46:02 sean-k-mooney ah ok that wold make sense then
12:46:42 sean-k-mooney i guess if any of them fail we dont realy care why we know the request is invlaid
12:47:27 songwenping yes, i also wonder if we should distinguish them
12:48:02 sean-k-mooney you proably do it today to have a better error message but you likely could do tha tdifferently
12:48:39 songwenping ack
12:49:54 sean-k-mooney oh https://github.com/openstack/nova/blob/35ddf1ad40207dee681a3c92cc9e86b061234edd/nova/db/sqlalchemy/api.py#L545-L550
12:50:18 sean-k-mooney so we do have that patteren
12:50:52 sean-k-mooney that would have changed form ServiceBinaryExists to ServiceTopicExists silently
12:52:03 songwenping so this also have problem
12:52:49 songwenping the tempest doesnot check the ServiceBinaryExists exception?
12:53:00 sean-k-mooney kind of becaue we use .get it wont fail
12:53:12 sean-k-mooney ya we likely dont have tempest coverage for this
12:53:23 sean-k-mooney although we should have and api funct tests for thsi
12:53:42 songwenping +1
12:53:47 gibi bauzas: OK, so you have plans to amend the audit support for MDEV. then that is really a good time to add some support for heal if possible.
12:54:19 bauzas gibi: well, I have around 12 hours for doing this until 3 weeks :p
12:54:27 songwenping i will commit one patch to coverage it.
12:54:34 bauzas 8 hours for tomorrow and 4 jours this afternoon :)
12:54:38 bauzas hours*
12:54:39 gibi bauzas: I'm not saying that you have to do it now. :D
12:54:56 gibi bauzas: it is totally ok to do it in Yoga
12:55:04 gibi bauzas: I just noted that we have a gap in heal
12:55:16 bauzas gibi: create a bug report and I'll own it
12:55:19 gibi I will make sure that it is now documented
12:55:27 gibi bauzas: OK I can file a bug too
12:55:30 bauzas or a blueprint maybe
12:55:31 gibi that is a good idea
12:55:45 gibi I will create a bug and later on we can transform that to a bp
12:55:50 bauzas this is a tech debt fix
12:55:51 gibi if we ever get to it
13:24:22 opendevreview Pierre Libeau proposed openstack/nova master: Reset state during service nova-compute init https://review.opendev.org/c/openstack/nova/+/803600

Earlier   Later