- tor-commits - lists.torproject.org

[sbws/master] fix: state: Read file before setting key
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit fe827ac182d7cee0061660d32dd56a7d11af3c65 Author: juga0 <juga(a)riseup.net> Date: Sat Mar 14 17:10:57 2020 +0000 fix: state: Read file before setting key Otherwise, if other instance of state set a key, it's lost by the current instance. Bugfix v0.7.0. --- sbws/util/state.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sbws/util/state.py b/sbws/util/state.py index 1d33c3d..f460413 100644 --- a/sbws/util/state.py +++ b/sbws/util/state.py @@ -97,6 +97,9 @@ class State: raise TypeError( 'May only store value with type in %s, not %s' % (State._ALLOWED_TYPES, type(value))) + # NOTE: important, read the file before setting the key, + # otherwise if other instances are creating other keys, they're lost. + self._state = self._read() self._state.__setitem__(key, value) self._write()

1 0

[sbws/master] fix: tests: Test state file consistency
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit 89efc2b6722c71b009ef3746a613b9fa512f01a8 Author: juga0 <juga(a)riseup.net> Date: Sat Mar 14 17:02:43 2020 +0000 fix: tests: Test state file consistency Test that two different instances of state don't overwrite each other. This test don't past in this commit, will pass in the next bugfix. Bugfix v0.7.0, which claimed 100% test coverage on state. --- tests/unit/util/test_state.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit/util/test_state.py b/tests/unit/util/test_state.py index dd23d1a..2443049 100644 --- a/tests/unit/util/test_state.py +++ b/tests/unit/util/test_state.py @@ -117,3 +117,12 @@ def test_state_iter(tmpdir): for key in d: state[key] = d[key] assert set([key for key in state]) == set(d) + + +def test_two_instances(tmpdir): + """Test that 2 different intances don't overwrite each other""" + s1 = State(os.path.join(str(tmpdir), 'state.dat')) + s2 = State(os.path.join(str(tmpdir), 'state.dat')) + s1["x"] = "foo" + s2["y"] = "bar" + assert s2["x"] == "foo"

1 0

[sbws/maint-1.1] fix: tests: Add tests loading results
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit fa1c0a4c6a101cdd62e93ee395ce3767fd1d6e53 Author: juga0 <juga(a)riseup.net> Date: Sun Mar 22 08:42:49 2020 +0000 fix: tests: Add tests loading results in ResultDump and incrementing relay's monitoring KeyValues. --- tests/unit/conftest.py | 12 ++----- tests/unit/lib/test_resultdump.py | 66 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index a74d03e..49db357 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -296,14 +296,6 @@ def end_event(): @pytest.fixture(scope='function') -def rd(args, conf, end_event): +def rd(args, conf_results): from sbws.lib.resultdump import ResultDump - # in Travis the next line gives the error: - # TypeError: __init__() takes 3 positional arguments but 4 were given - # No idea why. - # Returning None to disable the test in case ResultDump can not be - # initialized. - try: - return ResultDump(args, conf, end_event) - except TypeError: - return None + return ResultDump(args, conf_results) diff --git a/tests/unit/lib/test_resultdump.py b/tests/unit/lib/test_resultdump.py index c956dea..1180bdb 100644 --- a/tests/unit/lib/test_resultdump.py +++ b/tests/unit/lib/test_resultdump.py @@ -1,6 +1,16 @@ # -*- coding: utf-8 -*- """Unit tests for resultdump.""" -from sbws.lib.resultdump import trim_results_ip_changed + +import datetime + +from sbws.lib.relaylist import Relay +from sbws.lib.resultdump import ( + ResultError, + ResultErrorStream, + ResultSuccess, + trim_results_ip_changed, + load_result_file +) def test_trim_results_ip_changed_defaults(resultdict_ip_not_changed): @@ -31,3 +41,57 @@ def test_trim_results_ip_changed_on_changed_ipv6(caplog, assert record.levelname == 'WARNING' assert 'Reseting bandwidth results when IPv6 changes, ' \ 'is not yet implemented.\n' in caplog.text + + +def test_resultdump( + rd, args, conf_results, controller, router_status, server_descriptor +): + from sbws import settings + relay = Relay( + router_status.fingerprint, + controller, + ns=router_status, + desc=server_descriptor, + ) + relay.increment_relay_recent_priority_list() + relay.increment_relay_recent_measurement_attempt() + r = ResultSuccess( + [], 2000, relay, ["A", "B"], "http://localhost/bw", "scanner_nick", + ) + # Storing the result with `rd.queue.put` will not store the result to disk + # because the thread is not spawned with pytest. + rd.store_result(r) + results = rd.results_for_relay(relay) + # It has stored the result + assert 1 == len(results) + # The result has the correct attribute + assert 1 == len(results[0].relay_recent_priority_list) + # Store a second result for the sme relay + r = ResultError( + relay, ["A", "B"], "http://localhost/bw", "scanner_nick", + ) + rd.store_result(r) + assert 2 == len(results) + assert 1 == len(results[1].relay_recent_priority_list) + settings.set_end_event() + + +def test_load(datadir): + results = load_result_file(str(datadir.join("results.txt"))) + results = [v for values in results.values() for v in values] + r1 = results[1] + assert isinstance(r1, ResultSuccess) + assert isinstance( + r1.relay_recent_measurement_attempt[0], datetime.datetime + ) + assert 2 == len(r1.relay_recent_measurement_attempt) + assert 3 == len(r1.relay_recent_priority_list) + assert 3 == len(r1.relay_in_recent_consensus) + r2 = results[2] + assert isinstance(r2, ResultErrorStream) + assert isinstance( + r2.relay_recent_measurement_attempt[0], datetime.datetime + ) + assert 2 == len(r2.relay_recent_measurement_attempt) + assert 3 == len(r2.relay_recent_priority_list) + assert 3 == len(r2.relay_in_recent_consensus)

1 0

[sbws/maint-1.1] fix: tests: Check the files generated in test net
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit 4e2ccf3c34df54139954256528813305b48596c0 Author: juga0 <juga(a)riseup.net> Date: Sun Mar 22 16:28:16 2020 +0000 fix: tests: Check the files generated in test net Test that the results, state and bandwidth file generated by running the scanner and the generator in the test network are correct. --- tests/integration/conftest.py | 11 ++++----- tests/integration/test_files.py | 49 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index b1312d9..170ea51 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -36,15 +36,15 @@ def tmpdir(tmpdir_factory, request): @pytest.fixture(scope='session') def sbwshome_empty(tmpdir): - """Create sbws home inside of the tests tmp dir without initializing.""" - home = tmpdir.join('.sbws') - os.makedirs(home.strpath, exist_ok=True) - return home.strpath + """Create sbws home inside of the test net tmp dir without initializing.""" + home = "/tmp/.sbws" + os.makedirs(home, exist_ok=True) + return home @pytest.fixture(scope='session') def sbwshome_dir(sbwshome_empty): - """Create sbws home inside of the tests tmp dir without initializing.""" + """Create sbws home inside of the test net tmp dir without initializing.""" os.makedirs(os.path.join(sbwshome_empty, 'datadir'), exist_ok=True) return sbwshome_empty @@ -69,6 +69,7 @@ def conf(sbwshome_dir): """Default configuration with sbws home in the tmp test dir.""" conf = _get_default_config() conf['paths']['sbws_home'] = sbwshome_dir + conf["paths"]["state_fpath"] = os.path.join(sbwshome_dir, "state.dat") conf['tor']['run_dpath'] = os.path.join(sbwshome_dir, 'tor', 'run') conf['destinations']['foo'] = 'on' conf['destinations.foo'] = {} diff --git a/tests/integration/test_files.py b/tests/integration/test_files.py new file mode 100644 index 0000000..aa7973d --- /dev/null +++ b/tests/integration/test_files.py @@ -0,0 +1,49 @@ +""" +Integration tests for the files with data to be used by the bandwidth file. + +""" +from sbws.lib.resultdump import load_recent_results_in_datadir +from sbws.lib.v3bwfile import V3BWFile +from sbws.util.state import State + + +def test_results(conf): + results = load_recent_results_in_datadir(5, conf["paths"]["datadir"]) + for fp, values in results.items(): + count = max( + [ + len(getattr(r, "relay_recent_measurement_attempt", [])) + for r in values + ] + ) + assert count == 1 + count = max( + [len(getattr(r, "relay_in_recent_consensus", [])) for r in values] + ) + assert count == 1 + count = max( + [len(getattr(r, "relay_recent_priority_list", [])) for r in values] + ) + assert count == 1 + + +def test_state(conf): + state = State(conf["paths"]["state_fpath"]) + assert 1 == state.count("recent_consensus") + assert 1 == state.count("recent_priority_list") + assert 15 == state.count("recent_priority_relay") + assert 15 == state.count("recent_measurement_attempt") + + +def test_v3bwfile(conf): + bwfile = V3BWFile.from_v1_fpath( + conf["paths"]["v3bw_fname"].format("latest") + ) + assert "1" == bwfile.header.recent_consensus_count + assert "1" == bwfile.header.recent_priority_list_count + assert "15" == bwfile.header.recent_priority_relay_count + assert "15" == bwfile.header.recent_measurement_attempt_count + for bwline in bwfile.bw_lines: + assert 1 == bwline.relay_in_recent_consensus_count + assert 1 == bwline.relay_recent_priority_list_count + assert 1 == bwline.relay_recent_measurement_attempt_count

1 0

[sbws/maint-1.1] fix: v3bwfile: Stop calculating failures with 0 attempts
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit 1225c5d9ea738723e272288d6734dbc651cf2d7b Author: juga0 <juga(a)riseup.net> Date: Sun Mar 22 07:09:19 2020 +0000 fix: v3bwfile: Stop calculating failures with 0 attempts --- sbws/lib/v3bwfile.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sbws/lib/v3bwfile.py b/sbws/lib/v3bwfile.py index 6465892..245ffa6 100644 --- a/sbws/lib/v3bwfile.py +++ b/sbws/lib/v3bwfile.py @@ -379,7 +379,8 @@ class V3BWHeader(object): # failures = attempts - all mesaurements # Works only in the case that old measurements files already had # measurements count - if recent_measurement_attempt_count is not None: + # If this is None or 0, the failures can't be calculated + if recent_measurement_attempt_count: all_measurements = 0 for result_list in results.values(): all_measurements += len(result_list)

1 0

[sbws/maint-1.1] fix: tests: Add results incrementing relays'
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit 5c45e321b3ddb574fc204be2eb42c66a7bbedbe5 Author: juga0 <juga(a)riseup.net> Date: Sun Mar 22 08:36:54 2020 +0000 fix: tests: Add results incrementing relays' monitoring KeyValues. --- tests/unit/lib/test_results.py | 59 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/unit/lib/test_results.py b/tests/unit/lib/test_results.py index 3c2a52e..dccca09 100644 --- a/tests/unit/lib/test_results.py +++ b/tests/unit/lib/test_results.py @@ -9,6 +9,8 @@ from sbws.lib.resultdump import ResultErrorStream from sbws.lib.resultdump import _ResultType from tests.unit.globals import monotonic_time +from sbws.lib.relaylist import Relay + def test_Result(result): ''' @@ -232,3 +234,60 @@ def test_ResultErrorAuth_from_dict(time_mock): assert isinstance(r1, ResultErrorAuth) assert isinstance(r2, ResultErrorAuth) assert str(r1) == str(r2) + + +def test_relay_in_recent_consensus_count( + args, conf_results, controller, router_status, server_descriptor +): + relay = Relay( + router_status.fingerprint, + controller, + ns=router_status, + desc=server_descriptor, + ) + # Initialize the ResultSuccess as `measure_relay` does + r = ResultSuccess( + [], 2000, relay, ["A", "B"], "http://localhost/bw", "scanner_nick", + ) + assert 1 == len(r.relay_in_recent_consensus) + relay.update_relay_in_recent_consensus() + r = ResultSuccess( + [], 2000, relay, ["A", "B"], "http://localhost/bw", "scanner_nick", + ) + assert 2 == len(r.relay_in_recent_consensus) + + +def test_relay_recent_measurement_attempt_count( + args, conf_results, controller, router_status, server_descriptor +): + relay = Relay( + router_status.fingerprint, + controller, + ns=router_status, + desc=server_descriptor, + ) + relay.increment_relay_recent_measurement_attempt() + relay.increment_relay_recent_measurement_attempt() + # Initialize the ResultSuccess as `measure_relay` does + r = ResultSuccess( + [], 2000, relay, ["A", "B"], "http://localhost/bw", "scanner_nick", + ) + assert 2 == len(r.relay_recent_measurement_attempt) + + +def test_relay_recent_priority_list_count( + args, conf_results, controller, router_status, server_descriptor +): + relay = Relay( + router_status.fingerprint, + controller, + ns=router_status, + desc=server_descriptor, + ) + relay.increment_relay_recent_priority_list() + relay.increment_relay_recent_priority_list() + # Initialize the ResultSuccess as `measure_relay` does + r = ResultSuccess( + [], 2000, relay, ["A", "B"], "http://localhost/bw", "scanner_nick", + ) + assert 2 == len(r.relay_recent_priority_list)

1 0

[sbws/maint-1.1] fix: test: Check that log prints a number
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit 3efb686765812146b7d861e38412c37b2b0e7190 Author: juga0 <juga(a)riseup.net> Date: Mon Mar 23 09:21:50 2020 +0000 fix: test: Check that log prints a number and not a list of timestamps. --- sbws/lib/heartbeat.py | 2 +- tests/unit/lib/test_heartbeat.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/sbws/lib/heartbeat.py b/sbws/lib/heartbeat.py index b7b2387..1236890 100644 --- a/sbws/lib/heartbeat.py +++ b/sbws/lib/heartbeat.py @@ -45,7 +45,7 @@ class Heartbeat(object): Log the percentage, the number of relays measured and not measured, the number of loops and the time elapsed since it started measuring. """ - loops_count = self.state_dict.get('recent_priority_list_count', 0) + loops_count = self.state_dict.count('recent_priority_list') not_measured_fp_set = self.consensus_fp_set.difference( self.measured_fp_set diff --git a/tests/unit/lib/test_heartbeat.py b/tests/unit/lib/test_heartbeat.py index f3d8f52..b2ad634 100644 --- a/tests/unit/lib/test_heartbeat.py +++ b/tests/unit/lib/test_heartbeat.py @@ -2,9 +2,12 @@ import logging from sbws.lib import heartbeat +from sbws.util.state import State def test_total_measured_percent(conf, caplog): + state = State(conf["paths"]["state_fname"]) + state["recent_priority_list"] = [1, 2, 3] hbeat = heartbeat.Heartbeat(conf.getpath('paths', 'state_fname')) hbeat.register_consensus_fprs(['A', 'B', 'C']) @@ -19,6 +22,7 @@ def test_total_measured_percent(conf, caplog): hbeat.print_heartbeat_message() assert hbeat.previous_measurement_percent == 67 + assert 0 == caplog.records[0].getMessage().find("Run 3 main loops.") assert 0 == caplog.records[1].getMessage().find( "Measured in total 2 (67%)" )

1 0

[sbws/maint-1.1] fix: doc: Explain changes in the previous commits
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit 1e094500763636ff9f2b6238eb5ee04142c199ca Author: juga0 <juga(a)riseup.net> Date: Mon Mar 23 07:18:12 2020 +0000 fix: doc: Explain changes in the previous commits Closes: #30905. --- docs/source/implementation.rst | 92 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/docs/source/implementation.rst b/docs/source/implementation.rst index 7470b62..ad48a51 100644 --- a/docs/source/implementation.rst +++ b/docs/source/implementation.rst @@ -36,7 +36,7 @@ A first solution would be to obtain the git revision at runtime, but: the git revision of that other repository. So next solution was to obtain the git revision at build/install time. -To achive this, an script should be call from the installer or at runtime +To achive this, an script should be called from the installer or at runtime whenever `__version__` needs to be read. While it could be implemented by us, there're two external tools that achive @@ -95,4 +95,92 @@ git or python versions or we find a way to make `setuptools_scm` to detect the same version at buildtime and runtime. See `<https://github.com/MartinThoma/MartinThoma.github.io/blob/1235fcdecda4d71b4…>`_ -for other comparative versioning python packages. \ No newline at end of file +for other comparative versioning python packages. + + +Changing Bandwidth file monitoring KeyValues +-------------------------------------------- + +In version 1.1.0 we added KeyValues call ``recent_X_count`` and +``relay_X_count`` which implied to modify serveral parts of the code. + +We only stored numbers for simpliciy, but then the value of this numbers +accumulate over the time and there is no way to know to which number decrease +since some of the main objects are not recreated at runtime and do not have +attributes about when they were created or updated. +The relations between the object do no follow usual one-to-many or many-to-many +relationships either, to be able to induce some numbers from the related +objects. + +The only way we could think to solve this is to store list of timestamps, +instead of just numbers, as an attribute in the objects that need to store +some counting. + +Where the values of the keys come from? +``````````````````````````````````````` + +In the file system, there are only two types of files were these values can be +stored: +- the results files in ``datadir`` +- the ``state.dat`` file + +Because of the structure of the content in the results files, they can store +KeyValues for the relays, but not for the headers, which need to be stored in +the ``state.dat`` file. + +The classes that manage these KeyValues are: + +``RelayList``: + +- recent_consensus_count +- recent_measurement_attempt_count + +``RelayPrioritizer``: + +- recent_priority_list_count +- recent_priority_relay_count + +``Relay`` and ``Result``: + +- relay_in_recent_consensus_count +- relay_recent_measurement_attempt_count +- relay_recent_priority_list_count + +Transition from numbers to datetimes +```````````````````````````````````` + +The KeyValues named ``_count`` in the results and the state will be ignored +when sbws is restarted with this change, since they will be written without +``_count`` names in these files json . + +We could add code to count this in the transition to this version, but these +numbers are wrong anyway and we don't think it's worth the effort since they +will be correct after 5 days and they have been wrong for long time. + +Additionally ``recent_measurement_failure_count`` will be negative, since it's +calculated as ``recent_measurement_attempt_count`` minus all the results. +While the total number of results in the last 5 days is corrrect, the number of +the attempts won't be until 5 days have pass. + +Disadvantages +````````````` + +``sbws generate``, with 27795 measurement attempts takes 1min instead of a few +seconds. +The same happens with the ``RelayPrioritizer.best_priority``, though so far +that seems ok since it's a python generator in a thread and the measurements +start before it has calculated all the priorities. +The same happens with the ``ResultDump`` that read/write the data in a thread. + +Conclussion +``````````` + +All these changes required lot of effort and are not optimal. It was the way +we could correct and maintain 1.1.0 version. +If a 2.0 version happens, we highly recommend re-design the data structures to +use a database using a well maintained ORM library, which will avoid the +limitations of json files, errors in data types conversions and which is +optimized for the type of counting and statistics we aim to. + +.. note:: Documentation about a possible version 2.0 and the steps to change + the code from 1.X needs to be created.

1 0

[sbws/maint-1.1] fix: test: Assert that caplog messages were found
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit e9d5bb084aa0ba9a1e0aaac22b6732ffa3e40623 Author: juga0 <juga(a)riseup.net> Date: Mon Mar 23 09:19:08 2020 +0000 fix: test: Assert that caplog messages were found --- tests/unit/lib/test_heartbeat.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit/lib/test_heartbeat.py b/tests/unit/lib/test_heartbeat.py index 5c82a4b..f3d8f52 100644 --- a/tests/unit/lib/test_heartbeat.py +++ b/tests/unit/lib/test_heartbeat.py @@ -19,5 +19,9 @@ def test_total_measured_percent(conf, caplog): hbeat.print_heartbeat_message() assert hbeat.previous_measurement_percent == 67 - caplog.records[1].getMessage().find("Measured in total 2 (67%)") - caplog.records[2].getMessage().find("1 relays still not measured") + assert 0 == caplog.records[1].getMessage().find( + "Measured in total 2 (67%)" + ) + assert 0 == caplog.records[2].getMessage().find( + "1 relays still not measured" + )

1 0

[sbws/maint-1.1] Use freeze_time() in other parts of our tests, too
by juga＠torproject.org 14 Apr '20

14 Apr '20

commit 74ad2d91e38124e005fb3adc36ac38181ee155af Author: Georg Koppen <gk(a)torproject.org> Date: Thu Apr 2 07:48:22 2020 +0000 Use freeze_time() in other parts of our tests, too When using `_relays_with_flags()` and similar methods it's possible that tests start to hang without time freezing. See bug 33748 for more details. We work around this by providing the necessary `freeze_time()` calls meanwhile. --- tests/conftest.py | 4 +++- tests/unit/lib/test_relaylist.py | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 3ffa7bb..5d156f1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,6 +3,7 @@ import pytest import os.path from unittest import mock +from freezegun import freeze_time from stem import descriptor from sbws import settings @@ -120,7 +121,8 @@ def router_status(server_descriptor, router_statuses): @pytest.fixture(scope='function') def relay_list(args, conf, controller): """Returns a RelayList containing the Relays in the controller""" - return relaylist.RelayList(args, conf, controller) + with freeze_time("2020-02-29 10:00:00"): + return relaylist.RelayList(args, conf, controller) @pytest.fixture(scope='function') diff --git a/tests/unit/lib/test_relaylist.py b/tests/unit/lib/test_relaylist.py index 5561d60..88f9db5 100644 --- a/tests/unit/lib/test_relaylist.py +++ b/tests/unit/lib/test_relaylist.py @@ -71,9 +71,8 @@ def test_increment_recent_measurement_attempt(args, conf, controller): It also tests that the state file is updated correctly. """ state = State(conf['paths']['state_fpath']) - # For this test it does not matter that the consensus timestamps or relays - # are not correct. - relay_list = RelayList(args, conf, controller=controller, state=state) + with freeze_time("2020-02-29 10:00:00"): + relay_list = RelayList(args, conf, controller=controller, state=state) # The initial count is 0 and the state does not have that key. assert 0 == relay_list.recent_measurement_attempt_count assert not state.get("recent_measurement_attempt", None)

1 0