| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-05 | |||
| 11:53:26 | sean-k-mooney | if we se it uses a CUSTOM_CYBORG_THING | |
| 11:54:05 | sean-k-mooney | and we look at the RP tree for the host and see that is not on the root RP we know it need to be healed usign the nested RP inventory | |
| 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 | |