openstack/oslo.log
Code Issues Proposed changes

Remove the usage of the Eventlet debug feature from oslo.log.

Browse Source 
We initially started to work on this topic with the intention to solve
the problem of using the Eventlet hub_prevent_multiple_readers debug feature.
This feature is a debug convenience. This feature is not supported by the new
asyncio hub[1] of Eventlet. You can also check the documentation here[2] from eventlet. The problem with disabling this procedure is that
it exposes you to risk. Deactivation applies to the entire stack. If a project
uses oslo.log, for example nova, then it exposes all threads to concurrent access
on the process file descriptors.
As explained in the function's documentation[3], when several greenlets are reading from
the same socket, it's difficult to predict which greenlet will receive which data.
You really need to be sure of what you're doing to use this function. Also explained
in the raise condition added by this commit[4] and we have a reproducer[5] that exposes
the problem. According to our tests, removing this line does not change the fix provided.
It also seems that cinder no longer uses[6] logging native thread.

Related-Bug: #1983863

[1] https://review.opendev.org/c/openstack/governance/+/902585/17..18
[2] https://eventlet.readthedocs.io/en/latest/asyncio/migration.html#known-limitations-and-work-in-progress
[3] https://github.com/eventlet/eventlet/blob/master/eventlet/debug.py#L149
[4] cb7c8c0196
[5] https://github.com/4383/oslo.log/blob/reproducer2/reproducer.py
[6] https://bugs.launchpad.net/oslo.log/+bug/1983863/comments/18

Co-authored-by: Hervé Beraud <hberaud@redhat.com>
Change-Id: Ia4c8e82f957c4ef3c176246fea0bc0adab4f4dc4
This commit is contained in:
damani42 2024-05-07 12:03:59 +02:00
parent f05a852142
commit 8705f67bfb
3 changed files with 18 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
oslo_log/pipe_mutex.py
View File

@ -43,21 +43,6 @@ class PipeMutex(object):
self.owner = None
self.recursion_depth = 0
# Usually, it's an error to have multiple greenthreads all waiting
# to read the same file descriptor. It's often a sign of inadequate
# concurrency control; for example, if you have two greenthreads
# trying to use the same memcache connection, they'll end up writing
# interleaved garbage to the socket or stealing part of each others'
# responses.
#
# In this case, we have multiple greenthreads waiting on the same
# file descriptor by design. This lets greenthreads in real thread A
# wait with greenthreads in real thread B for the same mutex.
# Therefore, we must turn off eventlet's multiple-reader detection.
#
# It would be better to turn off multiple-reader detection for only
# our calls to trampoline(), but eventlet does not support that.
eventlet.debug.hub_prevent_multiple_readers(False)
def acquire(self, blocking=True):
"""Acquire the mutex.

14
oslo_log/tests/unit/test_pipe_mutex.py
View File

@ -20,6 +20,7 @@ from unittest import mock
import eventlet
from eventlet import debug as eventlet_debug
from eventlet import greenpool
from eventlet import tpool
from oslo_log import pipe_mutex
@ -35,7 +36,7 @@ def quiet_eventlet_exceptions():
class TestPipeMutex(unittest.TestCase):
"""From Swift's test/unit/common/test_utils.py"""
"""From Swift's test/unit/common/test_utils.py"""
def setUp(self):
self.mutex = pipe_mutex.PipeMutex()
@ -141,18 +142,11 @@ class TestPipeMutex(unittest.TestCase):
greenthread1 = eventlet.spawn(do_stuff)
greenthread2 = eventlet.spawn(do_stuff)
real_thread1 = eventlet.patcher.original('threading').Thread(
target=do_stuff)
real_thread1.start()
real_thread2 = eventlet.patcher.original('threading').Thread(
target=do_stuff)
real_thread2.start()
_ = tpool.execute(do_stuff)
_ = tpool.execute(do_stuff)
greenthread1.wait()
greenthread2.wait()
real_thread1.join()
real_thread2.join()
self.assertEqual(''.join(sequence), "<>" * 40)

14
releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml Normal file
View File

@ -0,0 +1,14 @@
---
fixes:
- |
Remove the usage of the Eventlet debug feature from oslo.log
Then hub_prevent_multiple_readers is a debug convenience. This
feature is a debug convenience. The problem with disabling this
procedure is that it exposes you to risk. Deactivation applies
to the entire stack. If a project uses oslo.log, for example nova,
then it exposes all threads to concurrent access on the process file
descriptors. When several greenlets are reading from the same socket,
it's difficult to predict which greenlet will receive which data. That
services using oslo.log should prefer using tpool rather than un-monkey
patched version of stdlib threading module which is not compatible with
eventlet.