[tor-commits] [sbws/master] destination: stop running twice usability tests

juga at torproject.org juga at torproject.org
Wed Feb 27 09:11:30 UTC 2019


commit d33ac8f6d23b8b30664311b205a87d9a55e4c1d1
Author: juga0 <juga at riseup.net>
Date:   Tue Dec 18 11:33:54 2018 +0000

    destination: stop running twice usability tests
    
    in every measurement.
    This removes the need for an extra lock for every measurement
    It should also not be depending on a time interval, but on the
    number of failures detected.
    Not counting number of failures since it would need to modify the
    destination or list of at runtime. It should be done in a future
    refactor.
    
    Fixes bug #28897. Bugfix v0.3.0
---
 sbws/core/scanner.py                      |  9 +++--
 sbws/lib/destination.py                   | 36 +++++++-----------
 tests/integration/lib/test_destination.py | 61 +++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 27 deletions(-)

diff --git a/sbws/core/scanner.py b/sbws/core/scanner.py
index 78cc165..dd4198c 100644
--- a/sbws/core/scanner.py
+++ b/sbws/core/scanner.py
@@ -11,7 +11,8 @@ from ..lib.resultdump import ResultSuccess, ResultErrorCircuit
 from ..lib.resultdump import ResultErrorStream
 from ..lib.relaylist import RelayList
 from ..lib.relayprioritizer import RelayPrioritizer
-from ..lib.destination import DestinationList
+from ..lib.destination import (DestinationList,
+                               connect_to_destination_over_circuit)
 from ..util.timestamp import now_isodt_str
 from ..util.state import State
 from sbws.globals import fail_hard, HTTP_GET_HEADERS, TIMEOUT_MEASUREMENTS
@@ -284,9 +285,9 @@ def measure_relay(args, conf, destinations, cb, rl, relay):
         ]
     log.debug('Built circuit with path %s (%s) to measure %s (%s)',
               circ_fps, nicknames, relay.fingerprint, relay.nickname)
-    # Make a connection to the destionation webserver and make sure it can
-    # still help us measure
-    is_usable, usable_data = dest.is_usable(circ_id, s, cb.controller)
+    # Make a connection to the destination
+    is_usable, usable_data = connect_to_destination_over_circuit(
+        dest, circ_id, s, cb.controller, dest._max_dl)
     if not is_usable:
         log.debug('Destination %s unusable via circuit %s (%s), %s',
                   dest.url, circ_fps, nicknames, usable_data)
diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py
index 7bd64e8..468b3f6 100644
--- a/sbws/lib/destination.py
+++ b/sbws/lib/destination.py
@@ -163,7 +163,8 @@ class DestinationList:
         self._cb = circuit_builder
         self._rl = relay_list
         self._all_dests = dests
-        self._usable_dests = []
+        # Inially all destionations are usable until proven otherwise.
+        self._usable_dests = self._all_dests
         self._last_usability_test = 0
         self._usability_test_interval = \
             conf.getint('destinations', 'usability_test_interval')
@@ -172,8 +173,10 @@ class DestinationList:
         self._usability_lock = RLock()
 
     def _should_perform_usability_test(self):
-        return self._last_usability_test + self._usability_test_interval <\
-            time.time()
+        # Until bigger refactor, do not perform usability test.
+        # Every time a measurement is done, it already performs what usability
+        # test does.
+        return False
 
     def _perform_usability_test(self):
         self._usability_lock.acquire()
@@ -250,23 +253,10 @@ class DestinationList:
         '''
         Returns the next destination that should be used in a measurement
         '''
-        with self._usability_lock:
-            while True:
-                if self._should_perform_usability_test():
-                    self._perform_usability_test()
-                    log.debug('%s/%s of our configured destinations are '
-                              'usable at this time', len(self._usable_dests),
-                              len(self._all_dests))
-                if len(self._usable_dests) > 0:
-                    break
-                time_till_next_check = self._usability_test_interval + 0.0001
-                log.warning(
-                    'Of our %d configured destinations, none are usable at '
-                    'this time. Sleeping %f seconds on this blocking call '
-                    'to DestinationList.next() until we can check for a '
-                    'usable destination again.', len(self._all_dests),
-                    time_till_next_check)
-                time.sleep(time_till_next_check)
-
-        self._rng.shuffle(self._usable_dests)
-        return self._usable_dests[0]
+        # Do not perform usability tests since a destination is already proven
+        # usable or not in every measurement, and it should depend on a X
+        # number of failures.
+        # This removes the need for an extra lock for every measurement.
+        # Do not change the order of the destinations, just return a
+        # destination.
+        return self._rng.choice(self._usable_dests)
diff --git a/tests/integration/lib/test_destination.py b/tests/integration/lib/test_destination.py
new file mode 100644
index 0000000..53c0ab5
--- /dev/null
+++ b/tests/integration/lib/test_destination.py
@@ -0,0 +1,61 @@
+"""Integration tests for destination.py"""
+import sbws.util.requests as requests_utils
+from sbws.lib.destination import (DestinationList, Destination,
+                                  connect_to_destination_over_circuit)
+
+
+def test_destination_list_no_usability_test_success(
+        conf, persistent_launch_tor, cb, rl
+        ):
+    # In a future refactor, if DestionationList is not initialized with the
+    # controller, this test should be an unit test.
+    destination_list, error_msg = DestinationList.from_config(
+        conf, cb, rl, persistent_launch_tor
+        )
+    # Initially all destinations should be "usable".
+    assert destination_list._all_dests == destination_list._usable_dests
+    # Because this is disabled.
+    assert destination_list._should_perform_usability_test() is False
+    # Because there's only 1 destination in conftest, random should return
+    # the same one.
+    assert destination_list.next() == \
+        destination_list._all_dests[0]
+
+
+def test_connect_to_destination_over_circuit_success(persistent_launch_tor,
+                                                     dests, cb, rl):
+    destination = dests.next()
+    session = requests_utils.make_session(persistent_launch_tor, 10)
+    # Choose a relay that is not an exit
+    relay = [r for r in rl.relays
+             if r.nickname == 'relay1mbyteMAB'][0]
+    # Choose an exit, for this test it does not matter the bandwidth
+    helper = rl.exits_not_bad_allowing_port(destination.port)[0]
+    circuit_path = [relay.fingerprint, helper.fingerprint]
+    # build a circuit
+    circuit_id = cb.build_circuit(circuit_path)
+    # Perform "usability test"
+    is_usable, response = connect_to_destination_over_circuit(
+        destination, circuit_id, session, persistent_launch_tor, 1024)
+    assert is_usable is True
+    assert 'content_length' in response
+
+
+def test_connect_to_destination_over_circuit_fail(persistent_launch_tor,
+                                                  dests, cb, rl):
+    bad_destination = Destination('https://example.example', 1024, False)
+    # dests._all_dests.append(bad_destination)
+    # dests._usable_dests.append(bad_destination)
+    session = requests_utils.make_session(persistent_launch_tor, 10)
+    # Choose a relay that is not an exit
+    relay = [r for r in rl.relays
+             if r.nickname == 'relay1mbyteMAB'][0]
+    # Choose an exit, for this test it does not matter the bandwidth
+    helper = rl.exits_not_bad_allowing_port(bad_destination.port)[0]
+    circuit_path = [relay.fingerprint, helper.fingerprint]
+    # Build a circuit.
+    circuit_id = cb.build_circuit(circuit_path)
+    # Perform "usability test"
+    is_usable, response = connect_to_destination_over_circuit(
+        bad_destination, circuit_id, session, persistent_launch_tor, 1024)
+    assert is_usable is False





More information about the tor-commits mailing list