Commit d33ac8f6 authored by juga's avatar juga
Browse files

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
parent cacbea71

sbws/core/scanner.py

+5 −4
Original line number Diff line number Diff line
@@ -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)

sbws/lib/destination.py

+13 −23
Original line number Diff line number Diff line
@@ -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)

tests/integration/lib/test_destination.py

0 → 100644
+61 −0
Original line number Diff line number Diff line 
"""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