| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-02 | |||
| 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 :) | |
| 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? | |
| 17:55:02 | melwitt | nvm, reading backscroll and saw mention | |
| 17:56:04 | gibi | melwitt: o/ when you found https://github.com/eventlet/eventlet/issues/731#issuecomment-969891721 where the thread.Thread was called? | |
| 17:57:11 | melwitt | gibi: that isn't what I found directly and tbh I need to trace it again in case I made a mistake, but what I found in oslo.messaging is that *it* uses spawn_n in eventlet mode. but I think sean's patch would solve that right? | |
| 17:57:48 | gibi | we need to try probably | |
| 17:58:02 | gibi | it depends how oslo.messaging actaully calls spawn_n | |
| 17:58:17 | melwitt | doing s/spawn_n/spawn/ in our code wouldn't be enough but I think monkey patching the whole thing should be if I'm not missing something. let me see if I can find a link real quick | |
| 18:00:57 | melwitt | urgh, I remembering now that it was more complicated than that. I'll try to find where I saw thread.Thread. I wish I had put code links in the comment | |
| 18:00:57 | gibi | melwitt: no need to rush, my brain is already toasted :) | |
| 18:01:37 | melwitt | hehe ok | |
| 18:04:38 | melwitt | I'll find whatever it was and add comment on the bug | |
| 18:07:33 | gibi | thank you | |
| 18:08:01 | gibi | your original eventlet issue thread was a really good information source already. thank you for that too | |
| 23:29:22 | melwitt | gibi, sean-k-mooney, bauzas: just fyi monday is a holiday in the US and I will be back tuesday | |
| #openstack-nova - 2022-09-03 | |||
| 01:22:54 | opendevreview | Takashi Natsume proposed openstack/nova master: Update compute rpc version alias for zed https://review.opendev.org/c/openstack/nova/+/855706 | |
| 01:44:21 | opendevreview | Takashi Natsume proposed openstack/nova master: doc: mark the max microversion for zed https://review.opendev.org/c/openstack/nova/+/855707 | |
| 15:27:20 | opendevreview | Balazs Gibizer proposed openstack/nova master: Fix fair internal lock used from eventlet.spawn_n https://review.opendev.org/c/openstack/nova/+/855717 | |
| 15:31:21 | opendevreview | Balazs Gibizer proposed openstack/nova stable/yoga: Fix fair internal lock used from eventlet.spawn_n https://review.opendev.org/c/openstack/nova/+/855718 | |
| 15:37:14 | gibi | OK, so from nova perspective only master(zed), and yoga is affected. On xena we use fasteners 0.14.1 which has the original workaround | |
| 16:07:48 | opendevreview | Balazs Gibizer proposed openstack/nova master: Fix fair internal lock used from eventlet.spawn_n https://review.opendev.org/c/openstack/nova/+/855717 | |
| 16:08:31 | opendevreview | Balazs Gibizer proposed openstack/nova stable/yoga: Fix fair internal lock used from eventlet.spawn_n https://review.opendev.org/c/openstack/nova/+/855718 | |
| 20:33:16 | opendevreview | Elod Illes proposed openstack/os-vif master: DNM: dummy change to test gate health https://review.opendev.org/c/openstack/os-vif/+/855742 | |