| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-05 | |||
| 08:59:11 | gibi | as we use attribute access everywhere | |
| 08:59:20 | gibi | except in that one func test that now fails | |
| 09:00:02 | gibi | hm, no | |
| 09:00:09 | gibi | that caches is broken | |
| 09:00:14 | gibi | as sometimes we store dicts | |
| 09:00:19 | gibi | sometimes we store Row objects | |
| 09:00:37 | gibi | https://github.com/openstack/placement/blob/13bbdba06da19f85c05a2a9e1fbdb9d1813c3b47/placement/attribute_cache.py#L163 | |
| 09:01:16 | sean-k-mooney[m] | https://github.com/openstack/placement/blob/48f31d446be5dd8743392e6d1e45ed8183a9ce1b/placement/attribute_cache.py#L148-L151 | |
| 09:01:54 | sean-k-mooney[m] | we store db rows when we load it form the db | |
| 09:02:05 | gibi | yep so this is a type mess | |
| 09:03:05 | sean-k-mooney[m] | ya stephen was complainging about this in a differnt patch | |
| 09:04:55 | sean-k-mooney[m] | so the all_cache | |
| 09:05:00 | sean-k-mooney[m] | currently had the row | |
| 09:05:09 | sean-k-mooney[m] | but that could jsut be a new tuple firht | |
| 09:05:47 | sean-k-mooney[m] | https://github.com/openstack/placement/blob/48f31d446be5dd8743392e6d1e45ed8183a9ce1b/placement/attribute_cache.py#L151 | |
| 09:06:38 | sean-k-mooney[m] | self._all_cache = {r[1]: r for r in res} -> self._all_cache = {r[1]: (r[0], r[1]) for r in res} | |
| 09:06:46 | gibi | so while 'self._all_cache = {r[1]: r._mapping for r in res}' fixed the currently failing test case it breaks a bunch of gabbi tests | |
| 09:07:31 | sean-k-mooney[m] | what elese is in that row beyond the id and string | |
| 09:08:05 | gibi | updated_at and created_at | |
| 09:08:31 | gibi | it is in the select above the fetachall | |
| 09:08:46 | sean-k-mooney[m] | ah base is the time stamped mixin | |
| 09:09:35 | sean-k-mooney[m] | also the value of the cache is ment to be a dict not a tuple so my version is incorrect | |
| 09:10:14 | gibi | yeah I have to figure out while the gabbi tests fails with my change | |
| 09:10:32 | gibi | technically Row._mapping is not a dict just a dict like object | |
| 09:10:36 | gibi | so it might be a problem | |
| 09:10:55 | sean-k-mooney[m] | do you want to do dict(**r._mappings) | |
| 09:11:12 | sean-k-mooney[m] | by the way to make this dicts not rows | |
| 09:11:13 | gibi | yeah I can try that | |
| 09:11:37 | sean-k-mooney[m] | although if we are expecting rows and using .id | |
| 09:11:48 | sean-k-mooney[m] | you would need a named tuple instead | |
| 09:12:47 | gibi | that cache stores dicts so if there is .id access now that would fail anyhow | |
| 09:12:50 | sean-k-mooney[m] | named tuple is the only “standard” class that will give you the field and dict style access | |
| 09:13:12 | sean-k-mooney[m] | well its storing row objects currently | |
| 09:13:20 | sean-k-mooney[m] | its ment to ba a dict | |
| 09:14:05 | gibi | namedtuple does not give you dict access | |
| 09:14:22 | gibi | it sometimes store Row sometimes store Dict | |
| 09:14:36 | gibi | https://github.com/openstack/placement/blob/13bbdba06da19f85c05a2a9e1fbdb9d1813c3b47/placement/attribute_cache.py#L184-L189 | |
| 09:15:07 | gibi | so it seems we started using that cache also in a mixed mode | |
| 09:15:16 | gibi | at some places we use attribute acces on it | |
| 09:15:26 | gibi | hence the gabbi test failures | |
| 09:16:12 | sean-k-mooney[m] | https://github.com/openstack/placement/blame/13bbdba06da19f85c05a2a9e1fbdb9d1813c3b47/placement/objects/trait.py#L151 ya stephen added a fixme when they noticed that | |
| 09:16:44 | sean-k-mooney[m] | namedtuple give you indexed acces i.e. r[0] | |
| 09:16:58 | sean-k-mooney[m] | but i guess it wont give you r[‘id’] | |
| 09:17:18 | gibi | https://github.com/openstack/placement/blame/13bbdba06da19f85c05a2a9e1fbdb9d1813c3b47/placement/objects/resource_class.py#L71-L74 so we assumes Row here | |
| 09:18:35 | gibi | so we need to decide which was we go. a) Store Row (or namedtuple) objects in caches and keep attribute access b) store dict and go with dict access | |
| 09:18:37 | sean-k-mooney[m] | https://github.com/openstack/placement/commit/b3fe04f081a096258468d032560f46cdfe77e144 stpehen tried to remove these assumtions in ^ | |
| 09:19:07 | sean-k-mooney[m] | well that will work as a named tuple | |
| 09:19:28 | gibi | Row and namedtuple is pretty much compatible, yes | |
| 09:19:48 | sean-k-mooney[m] | i would avoid random dicts personally | |
| 09:19:50 | gibi | ack | |
| 09:20:07 | sean-k-mooney[m] | and either store the row or named tuple | |
| 09:20:14 | sean-k-mooney[m] | im not sure if there is a reason not to sotre thr row object | |
| 09:20:21 | sean-k-mooney[m] | does it increase memory | |
| 09:20:31 | sean-k-mooney[m] | or have any other sideffect we would not want | |
| 09:23:38 | gibi | storing row is OK the doc said dict hence my statement that the caches is broken | |
| 09:23:55 | sean-k-mooney[m] | ack | |
| 09:24:34 | gibi | we need to mix Row and namedtuple as I can only create namedtuple here https://github.com/openstack/placement/blob/13bbdba06da19f85c05a2a9e1fbdb9d1813c3b47/placement/attribute_cache.py#L157-L168 | |
| 09:24:40 | gibi | but they are compatible | |
| 09:24:43 | gibi | so I only leave a note | |
| 09:24:55 | gibi | when I convert that to namedtuple | |
| 09:39:38 | sean-k-mooney[m] | if you use a named tuple on line 155 as well in refersh_from_db i thik we dont need to mix types but cool ill review when you push | |
| 09:47:01 | songwenping_ | sean-k-mooney[m],gibi: hi, nova-scheduler get allocation_candidates return 504 gateway timeout when create vm with 8gpus requests on our client's env, there are 13 gpu compute, and every compute has 8gpus. | |
| 09:47:02 | opendevreview | Balazs Gibizer proposed openstack/placement master: Make us compatible with oslo.db 12.1.0 https://review.opendev.org/c/openstack/placement/+/855862 | |
| 09:47:07 | gibi | sean-k-mooney[m]: ^^ | |
| 09:47:12 | gibi | stephenfin: ^^ | |
| 09:50:49 | gibi | sean-k-mooney[m]: also here is my stab at the nova only fair lock fix https://review.opendev.org/c/openstack/nova/+/855717 there is the oslo version of the fix https://review.opendev.org/c/openstack/oslo.concurrency/+/855714 | |
| 09:51:01 | songwenping_ | i imitate to insert some test datas on my devstack env, and the result is same, perhaps the api of allocation_candidates/limit... need to optimize. | |
| 09:51:27 | gibi | but I have to go back and think about the unit tests as it seems they are unstable | |
| 09:53:13 | opendevreview | Amit Uniyal proposed openstack/nova master: Adds check for VM snapshot fail while quiesce https://review.opendev.org/c/openstack/nova/+/852171 | |
| 09:54:22 | sean-k-mooney[m] | songwenping_: this is physical gpu passthoug? | |
| 09:54:29 | sean-k-mooney[m] | not vGPU correct | |
| 09:54:30 | songwenping_ | yes | |
| 09:54:41 | songwenping_ | pgpu passthough | |
| 09:55:05 | sean-k-mooney[m] | those are not tracked in placment then | |
| 09:55:18 | auniyal_ | Hi | |
| 09:55:23 | auniyal_ | please review these | |
| 09:55:25 | auniyal_ | https://review.opendev.org/c/openstack/nova/+/854980 | |
| 09:55:25 | auniyal_ | https://review.opendev.org/c/openstack/nova/+/854979 | |
| 09:55:25 | auniyal_ | backporting | |
| 09:55:25 | auniyal_ | https://review.opendev.org/c/openstack/nova/+/854499 | |
| 09:55:25 | auniyal_ | https://review.opendev.org/c/openstack/nova/+/852171 | |
| 09:56:17 | sean-k-mooney[m] | songwenping_: on master we now can track pci devics in placment but in any other release pci devices are not tracked in placment | |
| 09:56:34 | songwenping_ | _ID_1DB6&required6=CUSTOM_GPU_NVIDIA%2CCUSTOM_GPU_PRODUCT_ID_1DB6&resources=MEMORY_MB%3A32%2CVCPU%3A1&resources1=PGPU%3A1&resources2=PGPU%3A1&resources3=PGPU%3A1&resources4=PGPU%3A1&resources5=PGPU%3A1&resources6=PGPU%3A1" -H "Accept: application/json" -H "OpenStack-API-Version: placement 1.29" -H "User-Agent: openstacksdk/0.99.0 keystoneauth1/4.6.0 python-requests/2.27.1 CPython/3.8.10" -H "X-Auth-Token: gAAAAABjFbkj8nz24Q7A6J0qmjpdZHfWM | |
| 09:56:34 | songwenping_ | sean-k-mooney[m]: we use cyborg to manage pgpu, the request url is :curl -g -i -X GET "http://10.7.20.73/placement/allocation_candidates?limit=1000&group_policy=none&required1=CUSTOM_GPU_NVIDIA%2CCUSTOM_GPU_PRODUCT_ID_1DB6&required2=CUSTOM_GPU_NVIDIA%2CCUSTOM_GPU_PRODUCT_ID_1DB6&required3=CUSTOM_GPU_NVIDIA%2CCUSTOM_GPU_PRODUCT_ID_1DB6&required4=CUSTOM_GPU_NVIDIA%2CCUSTOM_GPU_PRODUCT_ID_1DB6&required5=CUSTOM_GPU_NVIDIA%2CCUSTOM_GPU_PRODUCT | |
| 09:56:35 | songwenping_ | vZidJOT9iYhV2MQCngcYQHhSQmjsGJofkYoT087tAISpf3IniDGwPTHXz_-8x-1nF60WavSYFgEd-5l3_ENrGumaHuU1yfhMJqZu06IR4SXacjA1g6ImSSEfLbfQ9zrPouB0roFokHPmPy3-UpnFZE" | |
| 09:56:46 | gibi | sean-k-mooney[m], sean-k-mooney[m]: is it via cyborg? because then it might be tracked in placement | |
| 09:56:53 | sean-k-mooney[m] | ok | |
| 09:58:05 | sean-k-mooney[m] | in that case perhaps they are hitting the compintorial explosion we were worried about with tracking VFs directly | |
| 09:58:11 | gibi | probably there too many possible candidates | |
| 09:58:19 | sean-k-mooney[m] | there are 13 chose 8 combinations | |
| 09:58:39 | sean-k-mooney[m] | that 1287 | |
| 09:58:46 | gibi | that is not that much | |
| 09:58:55 | gibi | but if there is 1000 computes | |
| 09:58:59 | gibi | or just 100 | |
| 09:58:59 | sean-k-mooney[m] | 1287*1000 | |
| 09:59:07 | gibi | then that is sizeable | |
| 09:59:16 | sean-k-mooney[m] | ya it will grow quickly | |
| 10:00:03 | sean-k-mooney[m] | sorry no | |
| 10:00:18 | songwenping_ | there are extra 8 computes without gpu. | |
| 10:00:22 | sean-k-mooney[m] | its 13 hosts each with 8 gpus and the vm is asking for 8 | |