Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-09-02
16:57:46 sean-k-mooney doing the repacement the same way in yoru standalone repoducer does not help?
16:58:05 gibi it does but my reproducer does not use the Threading.thread way the last comment in the issue mentions
16:58:20 sean-k-mooney right but does the lock
16:58:21 gibi and we know from melwitt that someting under nova uses Threading.thread
16:58:34 sean-k-mooney that should not matter by the way
16:58:55 sean-k-mooney even if it deoes the global replacment should mean that provided that call did not happen before we monkey patched
16:59:05 sean-k-mooney it should get our replaced version
16:59:09 gibi nope
16:59:19 gibi the internall call does greenlet.spawn_n
16:59:39 gibi I think that is not even replaceble as it is a c extension
16:59:41 sean-k-mooney oh well we can patch that too
16:59:51 sean-k-mooney oh
16:59:56 sean-k-mooney if its c then no
17:00:15 sean-k-mooney but we dont uses threading.thread normally
17:00:35 sean-k-mooney so while it might not fix every case it might fix the case we care about
17:00:42 gibi melwitt found someting that uses
17:00:49 sean-k-mooney teh libvirt thread
17:01:01 gibi what the rpc worker use?
17:01:23 gibi how we spawn the rpc workers listening on rabbit?
17:01:28 sean-k-mooney whell there are only two thread i can think of that might use it
17:01:44 sean-k-mooney the libvirt one and the heathbeat
17:01:51 sean-k-mooney if we use a pthred
17:02:06 sean-k-mooney im not sure we are using one for rpc explictly but we might be
17:02:18 gibi note that threading.thread issue path is problematic when it is monkey patched
17:02:43 gibi so when threading.thread actually patched to create an eventlet
17:03:07 gibi I can try tracing our rpc eventlet to see if it is created from spawn_n or spawn
17:03:25 gibi I guess it is coming from oslo.messaging somehow
17:03:59 sean-k-mooney well we use it in a few places
17:04:31 sean-k-mooney https://github.com/openstack/nova/blob/18d9c85aa4cbdbc471c6c7916ca6f1367c7ab4e5/nova/virt/libvirt/host.py#L491
17:04:49 sean-k-mooney https://github.com/openstack/nova/blob/18d9c85aa4cbdbc471c6c7916ca6f1367c7ab4e5/nova/virt/hyperv/serialproxy.py#L101
17:05:08 sean-k-mooney i was going to use it for the healthchecks
17:05:20 gibi native threading is OK that is real python Thread
17:05:49 gibi https://github.com/openstack/nova/blob/18d9c85aa4cbdbc471c6c7916ca6f1367c7ab4e5/nova/virt/hyperv/serialproxy.py#L101 <-- this can be a problem though
17:06:07 gibi hm it is native too https://github.com/openstack/nova/blob/18d9c85aa4cbdbc471c6c7916ca6f1367c7ab4e5/nova/virt/hyperv/serialproxy.py#L32
17:06:31 gibi so this two places creates a real python thread that is probably OK from this issue perspective
17:07:32 sean-k-mooney so all other usages today are in libs
17:08:05 sean-k-mooney oslo.messaging, ovsdbapp (via os-vif), os-brick, and proably oslo.db
17:08:08 sean-k-mooney if i was to guess
17:08:37 sean-k-mooney https://github.com/openstack/os-brick/blob/4acfd6bc7e7e816ce8e9d5ac59cfc0f6e5e816f4/os_brick/executor.py#L71
17:08:48 gibi the rabbit driver has threading but I'm not sure if that is always native https://github.com/openstack/oslo.messaging/blob/e44f286ebca0fbde5eae2f7eb9a21ba55ba2a549/oslo_messaging/_drivers/impl_rabbit.py
17:09:13 sean-k-mooney gibi: its not we monkey patch before we import it
17:09:32 gibi OK so it might or might not native
17:09:33 gibi https://github.com/openstack/oslo.messaging/blob/e44f286ebca0fbde5eae2f7eb9a21ba55ba2a549/oslo_messaging/_drivers/impl_rabbit.py#L607
17:09:59 sean-k-mooney its sill be a GreenThread unless we disable patching threads
17:10:23 sean-k-mooney we currently mokeypatch every nova service
17:10:55 sean-k-mooney and that will happen very early
17:11:15 gibi OK, so if rabbit driver uses threading and it is monkeypatched then our rpc worker spawned by spawn_n and effect by the bug
17:11:22 gibi even if we replace eventlet.spawn_n
17:11:46 gibi I cannot do a reproduction in the functional as it does not use the rabbit driver
17:11:55 gibi so it won't be indicative
17:12:34 gibi doing a tempest reproducer would be a pita as the reproducer needs timing
17:12:53 sean-k-mooney we will need to re MonkeyPatch threading.Thread then
17:13:16 sean-k-mooney to make it not call greenlet.spwan_n
17:13:32 sean-k-mooney or change how ew do locking
17:13:46 sean-k-mooney do you knwo how/why it migh not be locking correctly
17:13:50 gibi or we need to do this globally https://review.opendev.org/c/openstack/nova/+/813114/4/nova/tests/fixtures/nova.py#425
17:14:07 gibi fastener changed how it get the current thread
17:14:34 gibi and getting it from a thread created by spawn_n actaully returns the native thread name not the eventlet id
17:14:54 gibi therefore fasteners sees to different eventelt as same
17:15:00 gibi and allow reentry to the lock
17:15:02 sean-k-mooney well that we can do
17:15:37 sean-k-mooney you should be able to test that in your repoducer
17:15:38 gibi the downside of the globlack monekeypatch that it might effect the native threads
17:16:05 sean-k-mooney that can un monky patch it
17:16:28 sean-k-mooney we can save the reference to the ortinal fucniton globally
17:16:48 sean-k-mooney adn restore it with a context manager when launching the native thread
17:17:01 gibi basically all the native thread user in our under nova needs to be aware of it
17:17:12 gibi *in or
17:17:28 gibi we can sure fix nova native threads
17:17:46 gibi I'm not sure we can fix native threading in all libs used by nova
17:17:53 sean-k-mooney well ok
17:17:58 sean-k-mooney we dont need to do it globally
17:18:02 sean-k-mooney we just need the lock to use it
17:18:18 gibi so we wrap the lock with a local mocking
17:18:18 sean-k-mooney so we only need to do that for oslo
17:18:26 sean-k-mooney basically
17:18:29 sean-k-mooney yes
17:18:33 gibi if we not pass the lock to libs then that could work
17:18:42 sean-k-mooney i dont think we do
17:18:47 gibi yeah
17:18:53 gibi so backtrack a bit
17:19:10 sean-k-mooney we also alrady have nova util fucntion for locks already
17:20:00 gibi how can we test it in a way that i) uses the real things like rabbit driver ii)
17:20:11 sean-k-mooney https://github.com/openstack/nova/blob/4ca795536521f5e3d65d44b4de63c25294a5e0c4/nova/utils.py#L63
17:20:14 gibi allows hacking timing to reproduce the problem
17:20:30 sean-k-mooney we just need to use threading.Thread right
17:22:04 sean-k-mooney or
17:22:06 gibi basically we need to make sure threading.current_thread points to eventlet.getcurrent if called in a patched thread
17:22:19 sean-k-mooney you could use my unix socket impl
17:23:19 sean-k-mooney ctully maybe not
17:23:38 gibi back to testing, I can probably add sleeps to nova running in devstack to simulat the timing but that is not something we can merge as a test
17:24:40 gibi ahh, actually it is haard. as I need timing right in scheduler and in the compute too
17:24:58 gibi the scheduler needs to run the two evac parallel enough to select the same host
17:24:59 sean-k-mooney ya i dint know if https://review.opendev.org/c/openstack/oslo.messaging/+/841892/4/oslo_messaging/tests/functional/notify/test_unix_socket.py will be able to trigger it
17:25:37 gibi I guess this does not use the rabbit driver for notifications
17:25:50 sean-k-mooney its my unix socket one
17:26:11 gibi yeah so with that I can prove that your unix impl is fixed
17:26:11 sean-k-mooney its using either the eventlet or threadpool executor
17:26:26 sean-k-mooney but i dont know if its broken
17:26:43 gibi true :)

Earlier   Later