| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2026-07-14 | |||
| 10:38:22 | gibi | yepp commenting out https://review.opendev.org/c/openstack/oslo.concurrency/+/988652/1/oslo_concurrency/lockutils.py#135 makes it not possible the reproduce the problem | |
| 10:38:36 | gibi | switching the bug to oslo.concurrency... | |
| 10:39:06 | sean-k-mooney | ya so the early out i thinkwas an attempt to not import it over and over | |
| 10:39:20 | sean-k-mooney | but python caches import so its cheap | |
| 10:39:42 | sean-k-mooney | if we want to cache somethin we coudl cache the result of eventlet_patcher.is_monkey_patched('thread') | |
| 10:39:57 | sean-k-mooney | but i would just delete it | |
| 10:40:18 | sean-k-mooney | we dont need the extra complications of _EVENTLET_CHECKED | |
| 10:40:47 | sean-k-mooney | brb coffee | |
| 10:44:42 | gibi | I need to craft a oslo.concurrency level of test for this to avoid future regressions | |
| 11:05:58 | sean-k-mooney | in this case just create 2 rwlocks when eventlet is monky patched and assert both have the same method fo self._current_thread | |
| 11:07:49 | sean-k-mooney | a better test might be to test the side effect | |
| 11:08:25 | sean-k-mooney | i.e. that they both agree on the current thread id | |
| 11:15:36 | gibi | yeah I want the latter as that does not depend on the implementation | |
| 11:15:43 | gibi | hm | |
| 11:15:45 | gibi | not event that | |
| 11:15:56 | gibi | I want a test that shows the mutual exlusiveness of a write lock | |
| 11:16:17 | sean-k-mooney | hum ya that is higher level and less coupled | |
| 11:16:41 | sean-k-mooney | well | |
| 11:17:02 | sean-k-mooney | 2 locks are indpemendent | |
| 11:17:32 | sean-k-mooney | so its not as simple as aquire lock a and lock b and try to access somethign shared | |
| 11:18:02 | sean-k-mooney | you need to create 2 locks | |
| 11:18:41 | sean-k-mooney | and aquire both in 2 diffent eventsletlest or somthing | |
| 11:18:53 | sean-k-mooney | ideally with less infram then nova test case | |
| 11:19:44 | gibi | hm. isn't it the second lock I create will not work if used from two different greenlet? | |
| 11:20:13 | sean-k-mooney | its not really that you need two locks | |
| 11:20:33 | sean-k-mooney | in this case it that the second time any RW lock was inited the relevent code was skipped | |
| 11:20:41 | gibi | yepp | |
| 11:20:50 | gibi | so the second lock is broken when shared across greenlets | |
| 11:20:53 | gibi | that is my theory | |
| 11:21:01 | sean-k-mooney | so for the workers you can just use 1 lock and a thread.event | |
| 11:21:17 | sean-k-mooney | gibi: yes thre is a rentrency issue | |
| 11:21:31 | sean-k-mooney | in that when shared the second lock seas the real treadid | |
| 11:21:42 | sean-k-mooney | not the evnetlet greenthread id | |
| 11:22:04 | sean-k-mooney | so it goes one im on the same trhead where i aquired the lock and it races | |
| 11:23:03 | sean-k-mooney | i think its breaking https://github.com/openstack/nova/blob/master/nova/utils.py#L1233 | |
| 11:24:00 | sean-k-mooney | hum although maybe not | |
| 11:24:08 | sean-k-mooney | sinc ei have test for that and those pass | |
| 11:24:23 | sean-k-mooney | it was only one test that actully failed right | |
| 11:24:44 | sean-k-mooney | so its not any fo the simple test case i am already testing | |
| 11:26:20 | sean-k-mooney | im going to stop guessing if you come up with a test that works let me know and i can take a look | |
| 11:31:21 | gibi | sure I'm working on it | |
| 11:31:59 | gibi | it is a bit more complicated due to https://github.com/openstack/oslo.concurrency/blob/9aff32494fb97b7fc7508269224353aceb9ceef6/oslo_concurrency/lockutils.py#L101-L103 | |
| 11:32:23 | gibi | so they are also passing threading.current_thread to the parent | |
| 11:32:49 | gibi | sorry I ment https://github.com/openstack/oslo.concurrency/blob/9aff32494fb97b7fc7508269224353aceb9ceef6/oslo_concurrency/lockutils.py#L101-L108 | |
| 11:33:30 | sean-k-mooney | ya i was looking at that functor | |
| 11:34:19 | sean-k-mooney | threading.current_thread should be monkeypatch to eventlet.getcurrent in general before we hit this code | |
| 11:34:34 | sean-k-mooney | if we are usign eventlet | |
| 11:35:44 | sean-k-mooney | https://github.com/openstack/oslo.concurrency/commit/02dee958453513bb8a5aaae905312d306c2d9083#diff-60d0bce7c34c2c70cf4e3ac63060779235ede2a247bd39d1188f8fedd6306b38R109 | |
| 11:35:54 | sean-k-mooney | so that defaulting was added in the typeing patch | |
| 11:36:27 | gibi | ahh I see it, I need to use spawn_n not spawn to reproduce it | |
| 11:36:28 | sean-k-mooney | and it proably should not be | |
| 11:36:40 | gibi | the original eventlet issue is only affecting spawn_n | |
| 11:36:40 | sean-k-mooney | oh | |
| 11:36:46 | gibi | not spawn | |
| 11:36:54 | gibi | thanks melwitt for the good bug report | |
| 11:37:12 | sean-k-mooney | i know that changes between a raw greenlet and a greenthread | |
| 11:37:19 | sean-k-mooney | is that really the level where it broke | |
| 11:38:07 | gibi | yepp | |
| 11:38:09 | sean-k-mooney | i tought i had a patch to remove all useage of spawn_n at one point | |
| 11:38:23 | gibi | that is a good point ^^ we need to look | |
| 11:38:36 | sean-k-mooney | i am :) | |
| 11:39:20 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/892722 | |
| 11:40:27 | sean-k-mooney | so yes i did but we went with https://review.opendev.org/c/openstack/nova/+/873061 | |
| 11:40:29 | sean-k-mooney | instead | |
| 11:40:59 | sean-k-mooney | it would still be an oslo bug | |
| 11:41:10 | sean-k-mooney | but ya we can likely simplify now | |
| 11:42:11 | sean-k-mooney | oh we already have | |
| 11:42:38 | sean-k-mooney | spwan is now a call to spawn on with the default executor | |
| 11:43:24 | sean-k-mooney | but i bet the futurist executor might be using spwan_n somewhere along the way | |
| 11:44:32 | sean-k-mooney | we have a hacking check to prevent raw usage of spawn and spwan_n https://github.com/openstack/nova/commit/6179854ae77960d4ae883349626069023b407cb0 | |
| 11:45:22 | sean-k-mooney | well and eventlet in genreal so any usage is likely not in nova | |
| 11:46:01 | sean-k-mooney | gibi: yep https://github.com/openstack/futurist/blob/6f870896819197ca323b3dd8af8f3b0389aff69c/futurist/_futures.py#L718 | |
| 11:46:55 | sean-k-mooney | gibi: so the GreenThreadPoolExecotor uses spwan_n whne it spins up new workers | |
| 11:47:12 | gibi | I will be so happy when we finally delete eventlet :) | |
| 11:47:42 | sean-k-mooney | hehe so that litrally made me lol | |
| 11:47:48 | sean-k-mooney | but i agree | |
| 11:48:33 | sean-k-mooney | gibi: that can help you create a test case but i assume you already have one now | |
| 11:53:50 | opendevreview | sean mooney proposed openstack/nova stable/2025.2: Make QEMU_IMG_LIMITS process limits configurable https://review.opendev.org/c/openstack/nova/+/997028 | |
| 11:54:30 | opendevreview | sean mooney proposed openstack/nova stable/2025.1: Make QEMU_IMG_LIMITS process limits configurable https://review.opendev.org/c/openstack/nova/+/997029 | |
| 12:02:42 | gibi | sean-k-mooney: here is the repro and the fix https://review.opendev.org/c/openstack/oslo.concurrency/+/997200 | |
| 12:05:18 | sean-k-mooney | oh thats the fix :) | |
| 12:05:25 | sean-k-mooney | i was like wait they alrady have a bug for this | |
| 12:05:50 | sean-k-mooney | i.e. reading "# This is bug https://bugs.launchpad.net/oslo.concurrency/+bug/2160596" | |
| 12:06:59 | gibi | :) | |
| 12:07:10 | gibi | sean-k-mooney: thanks for the help root causing this issue | |
| 12:07:47 | sean-k-mooney | no worries i wrote the test that was beign broken so i was interested in the resolution | |
| 12:08:05 | sean-k-mooney | i tried really hard to make the FairLockGuard robust | |
| 12:08:22 | sean-k-mooney | so i was concerned i missed somthing | |
| 12:08:43 | gibi | sean-k-mooney: yeah all that effort is in vain if eventlet breaks you | |
| 12:17:59 | sean-k-mooney | minior nit in the commit messge of the second patch | |
| 12:18:07 | sean-k-mooney | but the logic in both looks good to me | |
| 12:18:50 | gibi | fixed | |
| 12:20:20 | sean-k-mooney | :) | |
| 12:59:39 | opendevreview | Joan Gilabert proposed openstack/nova master: Add libvirt driver MDEV ARQ support for Cyborg https://review.opendev.org/c/openstack/nova/+/995840 | |
| 12:59:39 | opendevreview | Joan Gilabert proposed openstack/nova master: Report OWNER_NOVA trait on resource providers https://review.opendev.org/c/openstack/nova/+/994299 | |
| 13:10:55 | opendevreview | Merged openstack/nova master: Reproducer for https://bugs.launchpad.net/nova/+bug/2160224 https://review.opendev.org/c/openstack/nova/+/996756 | |
| 13:49:48 | opendevreview | Balazs Gibizer proposed openstack/nova master: Do not try to return empty attachment_id from API https://review.opendev.org/c/openstack/nova/+/996757 | |
| 13:52:37 | gibi | gmaan: sean-k-mooney: ^^ this is now ready to land ;) | |
| 13:53:38 | gibi | sean-k-mooney: also if you have a sec we have this small fix https://review.opendev.org/c/openstack/nova/+/996757/1 | |
| 13:54:01 | sean-k-mooney | isnt that te same link | |