| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-05 | |||
| 08:45:34 | sean-k-mooney[m] | frickler: thats just a deprecation warning not an actual failure. placemient is mostly sqlachemey 2.0 compatiable already | |
| 08:46:03 | sean-k-mooney[m] | looks like we have missed one change but that should not be hard to adress | |
| 08:46:22 | gibi | frickler, sean-k-mooney[m]: I'm on it | |
| 08:47:02 | frickler | sean-k-mooney[m]: yes, just mentioned it because if would block that requirements patch. certainly not a big thing compared to the other blockers there | |
| 08:47:12 | gibi | we intended to store dicts in a caches but we stored row objects instead | |
| 08:47:12 | frickler | s/if/it/ | |
| 08:48:30 | sean-k-mooney[m] | frickler: well as it stands its just breaking the test as we treat warnings as errors. i doubt its breaking placment at all | |
| 08:48:45 | sean-k-mooney[m] | but our cache is not working correctly so thats a latent bug | |
| 08:54:41 | sean-k-mooney[m] | gibi https://github.com/openstack/placement/commit/c68d472dca6619055579831ad5464042f745557a i guess we missed this usecase | |
| 08:55:09 | sean-k-mooney[m] | the test is useing the dict interface instead of the field interface | |
| 08:55:58 | gibi | yeah, good point, we can store row objects in our rc caches if we access the columns by attribute acces and not by dict access | |
| 08:56:04 | gibi | let me check that it works | |
| 08:56:10 | gibi | as it provides an easier solutions | |
| 08:56:57 | sean-k-mooney[m] | https://github.com/openstack/placement/blob/13bbdba06da19f85c05a2a9e1fbdb9d1813c3b47/placement/objects/resource_class.py#L220 we are tryign to use _mappings to convert them to dict on get all | |
| 08:57:23 | sean-k-mooney[m] | well not to convert the via a dict to a resouceclass which should be fine | |
| 08:58:04 | gibi | here we have Row objects in the caches https://github.com/openstack/placement/blob/13bbdba06da19f85c05a2a9e1fbdb9d1813c3b47/placement/attribute_cache.py#L155 | |
| 08:59:05 | gibi | which is actually fine | |
| 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/+/852171 | |
| 09:55:25 | auniyal_ | https://review.opendev.org/c/openstack/nova/+/854499 | |
| 09:55:25 | auniyal_ | backporting | |
| 09:55:25 | auniyal_ | https://review.opendev.org/c/openstack/nova/+/854979 | |
| 09:55:25 | auniyal_ | https://review.opendev.org/c/openstack/nova/+/854980 | |
| 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_ | 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 | |