Nir Soffer has uploaded a new change for review.

Change subject: protocoldetector: Fix race in protocoldetector tests
......................................................................

protocoldetector: Fix race in protocoldetector tests

We used to initialize _next_cleanup when starting the server loop in
another thread. This creates a race for the testing the cleanup logic.
The tests waits 2 cleanup intervals to ensure that the server wakes up
and clean up stale connections, but because the timeout was set on
another thread, the test could wake up before the server woke up.

Now we initialize _next_cleanup when the object is created, so we know
exactly when the server should wake up.

Change-Id: Ic51f502172ccaf960dd1d9d33f7410d23f7fc491
Signed-off-by: Nir Soffer <[email protected]>
---
M tests/protocoldetectorTests.py
M vdsm/protocoldetector.py
2 files changed, 12 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/37046/1

diff --git a/tests/protocoldetectorTests.py b/tests/protocoldetectorTests.py
index a1231d1..22b7052 100644
--- a/tests/protocoldetectorTests.py
+++ b/tests/protocoldetectorTests.py
@@ -32,6 +32,14 @@
 from testlib import VdsmTestCase, expandPermutations, permutations
 
 
+class TestingAcceptor(protocoldetector.MultiProtocolAcceptor):
+    """
+    Acceptor with shorter cleanup interval to make it practical to test the
+    cleanup logic.
+    """
+    CLEANUP_INTERVAL = 1.0
+
+
 class Detector(object):
     """
     A detector returning response to the client, so we can tell if detection
@@ -192,9 +200,8 @@
     # Helpers
 
     def start_acceptor(self, use_ssl):
-        self.acceptor = protocoldetector.MultiProtocolAcceptor(
+        self.acceptor = TestingAcceptor(
             '127.0.0.1', 0, sslctx=self.SSLCTX if use_ssl else None)
-        self.acceptor.CLEANUP_INTERVAL = 1
         self.acceptor.add_detector(Echo())
         self.acceptor.add_detector(Uppercase())
         self.acceptor_address = self.acceptor._socket.getsockname()
diff --git a/vdsm/protocoldetector.py b/vdsm/protocoldetector.py
index 0b8ff60..e4a818a 100644
--- a/vdsm/protocoldetector.py
+++ b/vdsm/protocoldetector.py
@@ -85,7 +85,9 @@
         self._poller.register(self._read_fd, select.POLLIN)
         self._pending_connections = {}
         self._handlers = []
-        self._next_cleanup = 0
+        # Initialize *before* starting to serve, to make it easier to test the
+        # cleanup logic.
+        self._next_cleanup = time.time() + self.CLEANUP_INTERVAL
         self._required_size = None
 
     @traceback(on=log.name)
@@ -93,7 +95,6 @@
         self.log.debug("Running")
         self._required_size = max(h.REQUIRED_SIZE for h in self._handlers)
         self.log.debug("Using required_size=%d", self._required_size)
-        self._next_cleanup = time.time() + self.CLEANUP_INTERVAL
         try:
             while True:
                 try:


-- 
To view, visit http://gerrit.ovirt.org/37046
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic51f502172ccaf960dd1d9d33f7410d23f7fc491
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to