Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-09-02
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 sean-k-mooney so we only need to do that for oslo
17:18:18 gibi so we wrap the lock with a local mocking
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 sean-k-mooney its using either the eventlet or threadpool executor
17:26:11 gibi yeah so with that I can prove that your unix impl is fixed
17:26:26 sean-k-mooney but i dont know if its broken
17:26:43 gibi true :)
17:26:48 sean-k-mooney https://review.opendev.org/c/openstack/oslo.messaging/+/841892/4/oslo_messaging/notify/_impl_unix_socket.py#100
17:27:02 sean-k-mooney so its dynamicaly getting the exectuor and i think the futurist
17:27:13 sean-k-mooney threadpool executor is using threading.Thread internally
17:27:18 sean-k-mooney but its a streach
17:27:39 gibi I have to drop soon so I will sleep on this
17:27:52 gibi but fixing this is probalby an RC blocker
17:28:09 sean-k-mooney i think the best way to proceed is with a syntetic fix
17:28:18 sean-k-mooney import synconise for nova utiles
17:28:23 sean-k-mooney and tweak it until it works
17:28:34 sean-k-mooney using the simpler repoducer you were creating
17:29:05 sean-k-mooney https://github.com/openstack/futurist/blob/master/futurist/_thread.py#L43
17:29:06 gibi we can test the fix with the simple repro I have, what we cannot test that such fix is applied to every places we need it in nova :)
17:29:39 sean-k-mooney well i was thinkign of fixing it in oslo eventurlly
17:29:43 sean-k-mooney just for that lock
17:29:51 gibi that is better
17:30:03 gibi we can assume everything uses lock from oslo
17:30:10 gibi at least within core openstack
17:30:14 sean-k-mooney yep
17:30:16 sean-k-mooney they should
17:30:43 sean-k-mooney so when my unix socket driver is runing with the trehad execurftor it is using threading.Thread
17:30:44 gibi ack that is a way forward
17:30:49 sean-k-mooney but its not currently locking
17:30:53 sean-k-mooney so i dont think that helps
17:31:15 gibi I will summarise what we have in the bug and target the bug to oslo too
17:31:23 sean-k-mooney but i think we could adapt your fucn test into and oslo.synconisation fucn test
17:31:26 sean-k-mooney for the lock
17:32:15 sean-k-mooney sorry oslo.concurrency func test
17:34:29 gibi yes, I think so too
17:34:54 opendevreview Merged openstack/placement master: Fix typo in schema https://review.opendev.org/c/openstack/placement/+/849348
17:39:28 gibi bauzas: I think this is an RC blocker https://bugs.launchpad.net/oslo.concurrency/+bug/1988311 I tagged it but it seem we don't have official zed-rc-potential tag yet
17:45:16 gibi sean-k-mooney: an alternative fix is to roll back to fasteners < 0.15.0
17:45:30 gibi but I'm not sure that is viable in the whole core openstack
17:53:48 melwitt are yall considering doing sean-k-mooney's patch to monkey patch all of our spawn_n with spawn?

Earlier   Later