From f1dde6e3f15d46d3fd89fdbcdd4d4f802cd94ccc Mon Sep 17 00:00:00 2001
From: Michael Johnson <johnsomor@gmail.com>
Date: Thu, 11 Jul 2024 14:32:42 +0000
Subject: [PATCH] Revert "Remove the usage of the Eventlet debug feature from
 oslo.log."

This reverts commit 8705f67bfbd733edf923003c0a5850e7a4a5735b.

Reason for revert: Reverting this change as it is causing failures across multiple projects (nova and designate at least). OpenSearch is showing 580 related failures in the last 24 hours. As the comment states, this was an intentional setting based on how this "mutex" is implemented. I think this needs to stay in the code until we can move oslo service off of eventlet.

Related-Bug: #2072627

Change-Id: I562dc23c6ca41dcef6f40127f071b8422f677a0f
---
 oslo_log/pipe_mutex.py                            | 15 +++++++++++++++
 oslo_log/tests/unit/test_pipe_mutex.py            | 14 ++++++++++----
 ...t_multiple_readers-usage-b23cea69a05cd6d4.yaml | 14 --------------
 3 files changed, 25 insertions(+), 18 deletions(-)
 delete mode 100644 releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml

diff --git a/oslo_log/pipe_mutex.py b/oslo_log/pipe_mutex.py
index 8fbab699..825735ed 100644
--- a/oslo_log/pipe_mutex.py
+++ b/oslo_log/pipe_mutex.py
@@ -43,6 +43,21 @@ 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.
diff --git a/oslo_log/tests/unit/test_pipe_mutex.py b/oslo_log/tests/unit/test_pipe_mutex.py
index 4ff5d4de..c7b75915 100644
--- a/oslo_log/tests/unit/test_pipe_mutex.py
+++ b/oslo_log/tests/unit/test_pipe_mutex.py
@@ -20,7 +20,6 @@ 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
 
@@ -36,7 +35,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()
 
@@ -142,11 +141,18 @@ class TestPipeMutex(unittest.TestCase):
         greenthread1 = eventlet.spawn(do_stuff)
         greenthread2 = eventlet.spawn(do_stuff)
 
-        _ = tpool.execute(do_stuff)
-        _ = tpool.execute(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()
 
         greenthread1.wait()
         greenthread2.wait()
+        real_thread1.join()
+        real_thread2.join()
 
         self.assertEqual(''.join(sequence), "<>" * 40)
 
diff --git a/releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml b/releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml
deleted file mode 100644
index 3b8d1d3b..00000000
--- a/releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml
+++ /dev/null
@@ -1,14 +0,0 @@
----
-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 risks <https://eventlet.readthedocs.io/en/latest/reference/api/eventlet.html#eventlet.debug.hub_prevent_multiple_readers>`_.
-    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.