From b0aaf5d806276bfa946422d016da8506cc48a209 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 18 Dec 2018 11:33:54 +0000 Subject: [PATCH 01/10] 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(-) create mode 100644 tests/integration/lib/test_destination.py diff --git a/sbws/core/scanner.py b/sbws/core/scanner.py index 80251ec2..39d94918 100644 --- a/sbws/core/scanner.py +++ b/sbws/core/scanner.py @@ -6,7 +6,8 @@ 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 @@ -206,9 +207,9 @@ def measure_relay(args, conf, destinations, cb, rl, relay): log.debug('Built circ %s %s for relay %s %s', circ_id, stem_utils.circuit_str(cb.controller, circ_id), relay.nickname, relay.fingerprint[0:8]) - # 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.warning('When measuring %s %s the destination seemed to have ' 'stopped being usable: %s', relay.nickname, diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index 1b4c1927..efd1902c 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -159,7 +159,8 @@ def __init__(self, conf, dests, circuit_builder, relay_list, controller): 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') @@ -168,8 +169,10 @@ def __init__(self, conf, dests, circuit_builder, relay_list, controller): 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() @@ -245,23 +248,10 @@ def next(self): ''' 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 00000000..53c0ab5b --- /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 From b66391f10da33e967599e0c042442566c3da519f Mon Sep 17 00:00:00 2001 From: juga0 Date: Thu, 20 Dec 2018 15:26:33 +0000 Subject: [PATCH 02/10] destination: record consecutive failures Add methods to store consecutive destination failures and retrieve the destinations that are still functional. Since destinations can fail because of Tor circuits, it's not count individual failures but consecutives one. Also exit with error if there are no functional destinations left. The maximum number of consecuitve failures is set to 10, but it may need to be changed depending on the percentage of circuits and requests that fail. --- sbws/core/scanner.py | 7 +-- sbws/globals.py | 4 ++ sbws/lib/destination.py | 48 ++++++++++++++++++- tests/integration/lib/test_destination.py | 36 ++++++++++++++ .../integration/lib/test_relayprioritizer.py | 1 + 5 files changed, 91 insertions(+), 5 deletions(-) diff --git a/sbws/core/scanner.py b/sbws/core/scanner.py index 39d94918..914edcaa 100644 --- a/sbws/core/scanner.py +++ b/sbws/core/scanner.py @@ -170,10 +170,11 @@ def measure_relay(args, conf, destinations, cb, rl, relay): cb.controller, conf.getfloat('general', 'http_timeout')) # Pick a destionation dest = destinations.next() + # If there is no any destination at this point, it can not continue. if not dest: - log.warning('Unable to get destination to measure %s %s', - relay.nickname, relay.fingerprint[0:8]) - return None + log.critical("There are not any functional destinations.") + # This should raise an error so that the caller can close the pool. + exit(1) # Pick a relay to help us measure the given relay. If the given relay is an # exit, then pick a non-exit. Otherwise pick an exit. helper = None diff --git a/sbws/globals.py b/sbws/globals.py index 2277850d..43bd8c1e 100644 --- a/sbws/globals.py +++ b/sbws/globals.py @@ -53,6 +53,10 @@ BW_LINE_SIZE = 510 +# This number might need adjusted depending on the percentage of circuits and +# HTTP requests failures. +MAXIMUM_NUMBER_DESTINATION_FAILURES = 10 + def fail_hard(*a, **kw): ''' Log something ... and then exit as fast as possible ''' diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index efd1902c..9aed1466 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -9,6 +9,8 @@ import sbws.util.stem as stem_utils import sbws.util.requests as requests_utils +from ..globals import MAXIMUM_NUMBER_DESTINATION_FAILURES + log = logging.getLogger(__name__) @@ -76,21 +78,26 @@ def connect_to_destination_over_circuit(dest, circ_id, session, cont, max_dl): head = requests_utils.head(session, dest.url, verify=dest.verify) except (requests.exceptions.ConnectionError, requests.exceptions.ReadTimeout) as e: + dest.set_failure return False, 'Could not connect to {} over circ {} {}: {}'.format( dest.url, circ_id, stem_utils.circuit_str(cont, circ_id), e) finally: stem_utils.remove_event_listener(cont, listener) if head.status_code != requests.codes.ok: + dest.set_failure return False, error_prefix + 'we expected HTTP code '\ '{} not {}'.format(requests.codes.ok, head.status_code) if 'content-length' not in head.headers: + dest.set_failure return False, error_prefix + 'we except the header Content-Length '\ - 'to exist in the response' + 'to exist in the response' content_length = int(head.headers['content-length']) if max_dl > content_length: + dest.set_failure return False, error_prefix + 'our maximum configured download size '\ 'is {} but the content is only {}'.format(max_dl, content_length) log.debug('Connected to %s over circuit %s', dest.url, circ_id) + dest.failed = False return True, {'content_length': content_length} @@ -103,6 +110,39 @@ def __init__(self, url, max_dl, verify): assert u.netloc self._url = u self._verify = verify + # Flag to record whether this destination failed in the last + # measurement. + # Failures can't happen if: + # - an HTTPS request can not be made over Tor + # (which might be the relays fault, not the destination being + # unreachable) + # - the destination does not support HTTP Range requests. + self.failed = False + self.consecutive_failures = 0 + + @property + def is_functional(self): + """ + Returns True if there has not been a number consecutive measurements. + Otherwise warn about it and return False. + + """ + if self.consecutive_failures > MAXIMUM_NUMBER_DESTINATION_FAILURES: + log.warning("Destination %s is not functional. Please check that " + "it is correct.", self._url) + return False + return True + + @property + def set_failure(self): + """Set failed to True and increase the number of consecutive failures. + Only if it also failed in the previous measuremnt. + + """ + # if it failed in the last measurement + if self.failed: + self.consecutive_failures += 1 + self.failed = True def is_usable(self, circ_id, session, cont): ''' Use **connect_to_destination_over_circuit** to determine if this @@ -168,6 +208,10 @@ def __init__(self, conf, dests, circuit_builder, relay_list, controller): conf.getfloat('general', 'http_timeout') self._usability_lock = RLock() + @property + def functional_destinations(self): + return [d for d in self._all_dests if d.is_functional] + def _should_perform_usability_test(self): # Until bigger refactor, do not perform usability test. # Every time a measurement is done, it already performs what usability @@ -254,4 +298,4 @@ def next(self): # 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) + return self._rng.choice(self.functional_destinations) diff --git a/tests/integration/lib/test_destination.py b/tests/integration/lib/test_destination.py index 53c0ab5b..110dcd8a 100644 --- a/tests/integration/lib/test_destination.py +++ b/tests/integration/lib/test_destination.py @@ -39,6 +39,9 @@ def test_connect_to_destination_over_circuit_success(persistent_launch_tor, destination, circuit_id, session, persistent_launch_tor, 1024) assert is_usable is True assert 'content_length' in response + assert not destination.failed + assert destination.consecutive_failures == 0 + assert destination.is_functional def test_connect_to_destination_over_circuit_fail(persistent_launch_tor, @@ -59,3 +62,36 @@ def test_connect_to_destination_over_circuit_fail(persistent_launch_tor, is_usable, response = connect_to_destination_over_circuit( bad_destination, circuit_id, session, persistent_launch_tor, 1024) assert is_usable is False + + # because it is the first time it fails, failures aren't count + assert bad_destination.failed + assert bad_destination.consecutive_failures == 0 + assert bad_destination.is_functional + + # fail twice in a row + is_usable, response = connect_to_destination_over_circuit( + bad_destination, circuit_id, session, persistent_launch_tor, 1024) + assert bad_destination.failed + assert bad_destination.consecutive_failures == 1 + assert bad_destination.is_functional + + +def test_functional_destinations(conf, cb, rl, persistent_launch_tor): + good_destination = Destination('https://127.0.0.1:28888', 1024, False) + # Mock that it failed before and just now, but it's still considered + # functional. + good_destination.consecutive_failures = 3 + good_destination.failed = True + bad_destination = Destination('https://example.example', 1024, False) + # Mock that it didn't fail now, but it already failed 11 consecutive + # times. + bad_destination.consecutive_failures = 11 + bad_destination.failed = False + # None of the arguments are used, move to unit tests when this get + # refactored + destination_list = DestinationList( + conf, [good_destination, bad_destination], cb, rl, + persistent_launch_tor) + expected_functional_destinations = [good_destination] + functional_destinations = destination_list.functional_destinations + assert expected_functional_destinations == functional_destinations diff --git a/tests/integration/lib/test_relayprioritizer.py b/tests/integration/lib/test_relayprioritizer.py index d5464b8e..518286b7 100644 --- a/tests/integration/lib/test_relayprioritizer.py +++ b/tests/integration/lib/test_relayprioritizer.py @@ -13,6 +13,7 @@ def static_time(value): def _build_result_for_relay(conf, rl, result_type, relay_nick, timestamp): relay = [r for r in rl.relays if r.nickname == relay_nick] + print(rl.relays) assert len(relay) == 1 relay = relay[0] other = [r for r in rl.relays if r.nickname != relay_nick][0] From e8742dbc7ceb90d87ebf943246be40409f0087e4 Mon Sep 17 00:00:00 2001 From: juga0 Date: Sat, 22 Dec 2018 09:18:57 +0000 Subject: [PATCH 03/10] destination: remove unused code --- sbws/lib/destination.py | 73 ------------------- tests/integration/lib/test_destination.py | 4 - .../integration/lib/test_relayprioritizer.py | 1 - 3 files changed, 78 deletions(-) diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index 9aed1466..47a11fa3 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -1,8 +1,5 @@ import logging import random -import time -import os -from threading import RLock import requests from urllib.parse import urlparse from stem.control import EventType @@ -144,17 +141,6 @@ def set_failure(self): self.consecutive_failures += 1 self.failed = True - def is_usable(self, circ_id, session, cont): - ''' Use **connect_to_destination_over_circuit** to determine if this - destination is usable and return what it returns. Just a small wrapper. - ''' - if not isinstance(self.verify, bool): - if not os.path.isfile(self.verify): - return False, '{} is believed to be a CA bundle file on disk '\ - 'but it does not exist'.format(self.verify) - return connect_to_destination_over_circuit( - self, circ_id, session, cont, self._max_dl) - @property def url(self): return self._url.geturl() @@ -199,70 +185,11 @@ def __init__(self, conf, dests, circuit_builder, relay_list, controller): self._cb = circuit_builder self._rl = relay_list self._all_dests = 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') - self._usability_test_timeout = \ - conf.getfloat('general', 'http_timeout') - self._usability_lock = RLock() @property def functional_destinations(self): return [d for d in self._all_dests if d.is_functional] - def _should_perform_usability_test(self): - # 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() - log.debug('Perform usability tests') - cont = self._cont - timeout = self._usability_test_timeout - session = requests_utils.make_session(cont, timeout) - usable_dests = [] - for dest in self._all_dests: - possible_exits = self._rl.exits_not_bad_allowing_port(dest.port) - # Keep the fastest 10% of exits, or 3, whichever is larger - num_keep = int(max(3, len(possible_exits) * 0.1)) - possible_exits = sorted( - possible_exits, key=lambda e: e.consensus_bandwidth, - reverse=True) - exits = possible_exits[0:num_keep] - if len(exits) < 1: - log.warning("There are no exits to perform usability tests.") - continue - # Try three times to build a circuit to test this destination - circ_id = None - for _ in range(0, 3): - # Pick a random exit - exit = self._rng.choice(exits) - circ_id = self._cb.build_circuit([None, exit.fingerprint]) - if circ_id: - break - if not circ_id: - log.warning('Unable to build a circuit to test the usability ' - 'of %s. Assuming it isn\'t usable.', dest.url) - continue - log.debug('Built circ %s %s to test usability of %s', circ_id, - stem_utils.circuit_str(cont, circ_id), dest.url) - is_usable, data = dest.is_usable(circ_id, session, cont) - if not is_usable: - log.warning(data) - self._cb.close_circuit(circ_id) - continue - assert is_usable - log.debug('%s seems usable so we will keep it', dest.url) - usable_dests.append(dest) - self._cb.close_circuit(circ_id) - self._usable_dests = usable_dests - self._last_usability_test = time.time() - self._usability_lock.release() - @staticmethod def from_config(conf, circuit_builder, relay_list, controller): assert 'destinations' in conf diff --git a/tests/integration/lib/test_destination.py b/tests/integration/lib/test_destination.py index 110dcd8a..87d36a3e 100644 --- a/tests/integration/lib/test_destination.py +++ b/tests/integration/lib/test_destination.py @@ -12,10 +12,6 @@ def test_destination_list_no_usability_test_success( 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() == \ diff --git a/tests/integration/lib/test_relayprioritizer.py b/tests/integration/lib/test_relayprioritizer.py index 518286b7..d5464b8e 100644 --- a/tests/integration/lib/test_relayprioritizer.py +++ b/tests/integration/lib/test_relayprioritizer.py @@ -13,7 +13,6 @@ def static_time(value): def _build_result_for_relay(conf, rl, result_type, relay_nick, timestamp): relay = [r for r in rl.relays if r.nickname == relay_nick] - print(rl.relays) assert len(relay) == 1 relay = relay[0] other = [r for r in rl.relays if r.nickname != relay_nick][0] From 064117aff11633d90d8e930c05c332384d1e4b9e Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 26 Feb 2019 07:52:17 +0000 Subject: [PATCH 04/10] fixup! destination: record consecutive failures --- sbws/lib/destination.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index 47a11fa3..f8776f29 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -75,22 +75,22 @@ def connect_to_destination_over_circuit(dest, circ_id, session, cont, max_dl): head = requests_utils.head(session, dest.url, verify=dest.verify) except (requests.exceptions.ConnectionError, requests.exceptions.ReadTimeout) as e: - dest.set_failure + dest.set_failure() return False, 'Could not connect to {} over circ {} {}: {}'.format( dest.url, circ_id, stem_utils.circuit_str(cont, circ_id), e) finally: stem_utils.remove_event_listener(cont, listener) if head.status_code != requests.codes.ok: - dest.set_failure + dest.set_failure() return False, error_prefix + 'we expected HTTP code '\ '{} not {}'.format(requests.codes.ok, head.status_code) if 'content-length' not in head.headers: - dest.set_failure + dest.set_failure() return False, error_prefix + 'we except the header Content-Length '\ 'to exist in the response' content_length = int(head.headers['content-length']) if max_dl > content_length: - dest.set_failure + dest.set_failure() return False, error_prefix + 'our maximum configured download size '\ 'is {} but the content is only {}'.format(max_dl, content_length) log.debug('Connected to %s over circuit %s', dest.url, circ_id) @@ -130,7 +130,6 @@ def is_functional(self): return False return True - @property def set_failure(self): """Set failed to True and increase the number of consecutive failures. Only if it also failed in the previous measuremnt. From 469beaeb070d953a06cd47ee35ec12c89a943933 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 26 Feb 2019 07:53:25 +0000 Subject: [PATCH 05/10] fixup! destination: record consecutive failures --- sbws/lib/destination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index f8776f29..29014d3e 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -109,7 +109,7 @@ def __init__(self, url, max_dl, verify): self._verify = verify # Flag to record whether this destination failed in the last # measurement. - # Failures can't happen if: + # Failures can happen if: # - an HTTPS request can not be made over Tor # (which might be the relays fault, not the destination being # unreachable) From a21dbdd1439077f7ee5410597cf33146e809eeea Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 26 Feb 2019 07:56:00 +0000 Subject: [PATCH 06/10] fixup! destination: record consecutive failures --- sbws/globals.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sbws/globals.py b/sbws/globals.py index 43bd8c1e..8a068cc0 100644 --- a/sbws/globals.py +++ b/sbws/globals.py @@ -55,7 +55,9 @@ # This number might need adjusted depending on the percentage of circuits and # HTTP requests failures. -MAXIMUM_NUMBER_DESTINATION_FAILURES = 10 +# While the scanner can not recover from some/all failing destionations, +# set a big number so that it continues trying. +MAXIMUM_NUMBER_DESTINATION_FAILURES = 100 def fail_hard(*a, **kw): From 4f3a9bebf14a0a818a5e23cdc2ac33b83b41b3a4 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 26 Feb 2019 09:02:36 +0000 Subject: [PATCH 07/10] fixup! destination: record consecutive failures --- DEPLOY.rst | 2 ++ docs/source/examples/sbws.example.ini | 7 ++++++- docs/source/man_sbws.ini.rst | 5 +++++ sbws/core/scanner.py | 4 +++- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/DEPLOY.rst b/DEPLOY.rst index e62e17f6..9629c75e 100644 --- a/DEPLOY.rst +++ b/DEPLOY.rst @@ -30,6 +30,8 @@ or ``_ (local build or Read the Docs). To run the ``scanner`` it is mandatory to create a configuration file with at least one ``destination``. +It is recommended to set several ``destination``s so that the ``scanner`` can +continue if one fails. If ``sbws`` is installed from the Debian package, then create a file in ``/etc/sbws/sbws.ini`` like in the following example: diff --git a/docs/source/examples/sbws.example.ini b/docs/source/examples/sbws.example.ini index 0130bdb6..2a620108 100644 --- a/docs/source/examples/sbws.example.ini +++ b/docs/source/examples/sbws.example.ini @@ -4,7 +4,12 @@ nickname = sbws_default [destinations] -# a destination can be disabled changing `on` by `off` +# With several destinations, the scanner can continue even if some of them +# fail, which can be caused by a network problem on their side. +# If all of them fail, the scanner will stop, which +# will happen if there is network problem on the scanner side. + +# A destination can be disabled changing `on` by `off` foo = on [destinations.foo] diff --git a/docs/source/man_sbws.ini.rst b/docs/source/man_sbws.ini.rst index 0ea98b51..de603d67 100644 --- a/docs/source/man_sbws.ini.rst +++ b/docs/source/man_sbws.ini.rst @@ -58,6 +58,11 @@ paths (Default ~/.sbws/log) destinations + + It is required to set at least one destination for the scanner to run. + It is recommended to set several destinations so that the scanner can + continue if one fails. + STR = {on, off} Name of destination. It is a name for the Web server from where to download files in order to measure bandwidths. diff --git a/sbws/core/scanner.py b/sbws/core/scanner.py index 914edcaa..be8680cb 100644 --- a/sbws/core/scanner.py +++ b/sbws/core/scanner.py @@ -172,7 +172,9 @@ def measure_relay(args, conf, destinations, cb, rl, relay): dest = destinations.next() # If there is no any destination at this point, it can not continue. if not dest: - log.critical("There are not any functional destinations.") + log.critical("There are not any functional destinations.\n" + "It is recommended to set several destinations so that " + "the scanner can continue if one fails.") # This should raise an error so that the caller can close the pool. exit(1) # Pick a relay to help us measure the given relay. If the given relay is an From 9604166c4f11c2f4fdba957eb7cc511d20759a73 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 26 Feb 2019 09:20:05 +0000 Subject: [PATCH 08/10] fixup! destination: record consecutive failures --- sbws/lib/destination.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index 29014d3e..b7cd1a9d 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -94,6 +94,12 @@ def connect_to_destination_over_circuit(dest, circ_id, session, cont, max_dl): return False, error_prefix + 'our maximum configured download size '\ 'is {} but the content is only {}'.format(max_dl, content_length) log.debug('Connected to %s over circuit %s', dest.url, circ_id) + # Any failure connecting to the destination will call set_failure, + # which will set `failed` to True and count consecutives failures. + # It can not be set at the start, to be able to know if it failed a + # a previous time, which is checked by set_failure. + # Future improvement: store a list or fixed size dequeue of timestamps + # when it fails. dest.failed = False return True, {'content_length': content_length} From 07aa86ac9627fc22731a64e67acae84db2772761 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 26 Feb 2019 10:07:30 +0000 Subject: [PATCH 09/10] fixup! destination: record consecutive failures --- sbws/lib/destination.py | 4 ++-- tests/integration/lib/test_destination.py | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index b7cd1a9d..28e85369 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -98,8 +98,8 @@ def connect_to_destination_over_circuit(dest, circ_id, session, cont, max_dl): # which will set `failed` to True and count consecutives failures. # It can not be set at the start, to be able to know if it failed a # a previous time, which is checked by set_failure. - # Future improvement: store a list or fixed size dequeue of timestamps - # when it fails. + # Future improvement: use a list to count consecutive failures + # or calculate it from the results. dest.failed = False return True, {'content_length': content_length} diff --git a/tests/integration/lib/test_destination.py b/tests/integration/lib/test_destination.py index 87d36a3e..cc8c7943 100644 --- a/tests/integration/lib/test_destination.py +++ b/tests/integration/lib/test_destination.py @@ -1,4 +1,5 @@ """Integration tests for destination.py""" +from sbws.globals import MAXIMUM_NUMBER_DESTINATION_FAILURES import sbws.util.requests as requests_utils from sbws.lib.destination import (DestinationList, Destination, connect_to_destination_over_circuit) @@ -81,7 +82,8 @@ def test_functional_destinations(conf, cb, rl, persistent_launch_tor): bad_destination = Destination('https://example.example', 1024, False) # Mock that it didn't fail now, but it already failed 11 consecutive # times. - bad_destination.consecutive_failures = 11 + bad_destination.consecutive_failures = \ + MAXIMUM_NUMBER_DESTINATION_FAILURES + 1 bad_destination.failed = False # None of the arguments are used, move to unit tests when this get # refactored From b4cac3158801cc80b852457aafa75b14247b33b9 Mon Sep 17 00:00:00 2001 From: juga0 Date: Tue, 26 Feb 2019 11:02:59 +0000 Subject: [PATCH 10/10] fixup! destination: remove unused code --- sbws/lib/destination.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sbws/lib/destination.py b/sbws/lib/destination.py index 28e85369..1b4779cb 100644 --- a/sbws/lib/destination.py +++ b/sbws/lib/destination.py @@ -4,7 +4,6 @@ from urllib.parse import urlparse from stem.control import EventType import sbws.util.stem as stem_utils -import sbws.util.requests as requests_utils from ..globals import MAXIMUM_NUMBER_DESTINATION_FAILURES