August 2018 - tor-commits - lists.torproject.org

[sbws/master] Remove test reading header from file
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit 15ad0ca597910e6343631a6e02dcac7e2f1f5332 Author: juga0 <juga(a)riseup.net> Date: Sat Jul 14 12:03:14 2018 +0000 Remove test reading header from file reading the whole file is already in other test and software version changes with every release --- tests/unit/lib/test_v3bwfile.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/unit/lib/test_v3bwfile.py b/tests/unit/lib/test_v3bwfile.py index 0af1834..01c84ce 100644 --- a/tests/unit/lib/test_v3bwfile.py +++ b/tests/unit/lib/test_v3bwfile.py @@ -80,18 +80,6 @@ def test_v3bwheader_from_text(): assert str(header_obj) == str(header) -def test_v3bwheader_from_file(datadir): - """Test header str with additional headers""" - header = V3BWHeader(timestamp_l, - file_created=file_created, - generator_started=generator_started, - earliest_bandwidth=earliest_bandwidth) - # at some point this should be read from conftest - text = datadir.read('v3bw/20180425_131057.v3bw') - h, _ = V3BWHeader.from_text_v110(text) - assert str(h) == str(header) - - def test_num_results_of_type(result_success, result_error_stream): assert num_results_of_type([result_success], 'success') == 1 assert num_results_of_type([result_error_stream], 'success') == 0

1 0

[sbws/master] Combine Changes headers in Unreleased section
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit a7816bafd435df04da2e9ec14b04bfb7af9ebd9d Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Mon Jul 16 14:19:51 2018 -0400 Combine Changes headers in Unreleased section --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfac048..ddd7f21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] -### Changed - -- Remove test data v3bw file and generate it from the same test. (#26736) - ### Added - Log line on start up with sbws version, platform info, and library versions @@ -19,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed - Document at which times should v3bw files be generated (#26740) +- Remove test data v3bw file and generate it from the same test. (#26736) ## [0.6.0] - 2018-07-11

1 0

[sbws/master] Add State class and 100% test coverage for it
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit d3e1025bacd796eabe5285206762a858bf6d867a Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Fri Jul 13 12:07:03 2018 -0400 Add State class and 100% test coverage for it GH: ref #166 --- sbws/util/state.py | 62 ++++++++++++++++++++++ tests/unit/util/test_state.py | 120 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+) diff --git a/sbws/util/state.py b/sbws/util/state.py new file mode 100644 index 0000000..58aef38 --- /dev/null +++ b/sbws/util/state.py @@ -0,0 +1,62 @@ +from sbws.util.filelock import FileLock +import os +import json + + +class State: + _ALLOWED_TYPES = (int, float, str, bool, type(None)) + + def __init__(self, fname): + self._fname = fname + self._state = self._read() + + def _read(self): + with FileLock(self._fname): + if not os.path.exists(self._fname): + return {} + with open(self._fname, 'rt') as fd: + return json.load(fd) + + def _write(self): + with FileLock(self._fname): + with open(self._fname, 'wt') as fd: + return json.dump(self._state, fd) + + def __len__(self): + self._state = self._read() + return self._state.__len__() + + def __getitem__(self, key): + if not isinstance(key, str): + raise TypeError( + 'Keys must be strings. %s is a %s' % (key, type(key))) + self._state = self._read() + return self._state.__getitem__(key) + + def __delitem__(self, key): + if not isinstance(key, str): + raise TypeError( + 'Keys must be strings. %s is a %s' % (key, type(key))) + self._state = self._read() + self._state.__delitem__(key) + self._write() + + def __setitem__(self, key, value): + if not isinstance(key, str): + raise TypeError( + 'Keys must be strings. %s is a %s' % (key, type(key))) + if type(value) not in State._ALLOWED_TYPES: + raise TypeError( + 'May only store value with type in %s, not %s' % + (State._ALLOWED_TYPES, type(value))) + self._state = self._read() + self._state.__setitem__(key, value) + self._write() + + def __iter__(self): + self._state = self._read() + return self._state.__iter__() + + def __contains__(self, item): + self._state = self._read() + return self._state.__contains__(item) diff --git a/tests/unit/util/test_state.py b/tests/unit/util/test_state.py new file mode 100644 index 0000000..42a6a8e --- /dev/null +++ b/tests/unit/util/test_state.py @@ -0,0 +1,120 @@ +from sbws.util.state import State +import os +# from tempfile import NamedTemporaryFile as NTF + + +def test_state_set_allowed_key_types(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + attempt_keys = ('k') + for key in attempt_keys: + state[key] = 4 + assert state[key] == 4 + + +def test_state_set_bad_key_types(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + attempt_keys = (15983, None, True, -1.2, [], {}, set()) + for key in attempt_keys: + try: + state[key] = 4 + except TypeError: + pass + else: + assert None, 'Should not have been able to use %s %s as a key' %\ + (key, type(key)) + try: + state[key] + except TypeError: + pass + else: + assert None, '%s %s is not a valid key type, so should have got '\ + 'TypeError when giving it' % (key, type(key)) + + +def test_state_set_allowed_value_types(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + attempt_vals = (15983, None, True, -1.2, 'loooooool') + for val in attempt_vals: + state['foo'] = val + assert state['foo'] == val + + +def test_state_set_bad_value_types(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + attempt_vals = ([], {}, set()) + for val in attempt_vals: + try: + state['foo'] = val + except TypeError: + pass + else: + assert None, 'Should not have been able to use %s %s as a value' %\ + (val, type(val)) + + +def test_state_del(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + d = {'a': 1, 'b': 2, 'c': 3, 'd': 4} + for key in d: + state[key] = d[key] + assert len(state) == len(d) + + del d['a'] + del state['a'] + assert len(state) == len(d) + for key in d: + assert d[key] == state[key] + + attempt_keys = (15983, None, True, -1.2, [], {}, set()) + for key in attempt_keys: + try: + del state[False] + except TypeError: + pass + else: + assert None, 'Should not have been allowed to delete %s %s '\ + 'because it is not a valid key type' % (key, type(key)) + + d['e'] = 5 + state['e'] = 5 + d['e'] = 5.5 + state['e'] = 5.5 + assert len(state) == len(d) + + +def test_state_get_len(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + d = {'a': 1, 'b': 2, 'c': 3, 'd': 4} + for key in d: + state[key] = d[key] + assert len(state) == len(d) + + del d['a'] + del state['a'] + assert len(state) == len(d) + + d['e'] = 5 + state['e'] = 5 + d['e'] = 5.5 + state['e'] = 5.5 + assert len(state) == len(d) + + +def test_state_contains(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + d = {'a': 1, 'b': 2, 'c': 3, 'd': 4} + for key in d: + state[key] = d[key] + assert 'a' in state + assert 'e' not in state + + +def test_state_iter(tmpdir): + state = State(os.path.join(tmpdir, 'statefoo')) + for key in state: + pass + d = {'a': 1, 'b': 2, 'c': 3, 'd': 4} + for key in d: + state[key] = d[key] + for key in state: + pass

1 0

[sbws/master] Update docs on the contents of ~/.sbws
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit 4b61fb170485fb215d46643645a04201597e33c8 Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Mon Jul 16 15:10:53 2018 -0400 Update docs on the contents of ~/.sbws --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 74da116..49ca638 100644 --- a/README.md +++ b/README.md @@ -131,11 +131,12 @@ In this directory you will find - `config.ini` The configuration file you should be editing if you want to modify sbws's behavior. -- `config.log.ini` The configuration file you should edit if you want to modify - how sbws logs. -- `datadir` Once your sbws scanner has started gathering results, it will dump +- `datadir/` Once your sbws scanner has started gathering results, it will dump them into this directory. Other sbws commands (such as generate and stats) read results from the files in this directory. +- `log/` If configured, this directory stores logs generated by all the sbws + commands in rotating log files. +- `v3bw/` This directory stores the v3bw files created with `sbws generate`. ## Running tests

1 0

[sbws/master] Put scanner start time in state file ...
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit 38799a012055755fb10c9919bc4ae5524a358cf3 Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Mon Jul 16 14:52:45 2018 -0400 Put scanner start time in state file ... - Remove started_at file in config et al. - Add state_fname to config et al. - Remove read/write functions for the started_at file in scanner.py and genearte.py --- CHANGELOG.md | 2 ++ sbws/config.default.ini | 2 +- sbws/core/scanner.py | 22 ++++------------------ sbws/lib/v3bwfile.py | 30 +++++++----------------------- sbws/util/config.py | 4 ++-- 5 files changed, 16 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddd7f21..9a5ceee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Log line on start up with sbws version, platform info, and library versions (trac#26751) +- State file for storing the last time `sbws scanner` was started, and able to + be used for storing many other types of state in the future. (GH#166) ### Changed diff --git a/sbws/config.default.ini b/sbws/config.default.ini index b21ba0b..121154d 100644 --- a/sbws/config.default.ini +++ b/sbws/config.default.ini @@ -2,7 +2,7 @@ datadir = ${sbws_home}/datadir v3bw_dname = ${sbws_home}/v3bw v3bw_fname = ${v3bw_dname}/{}.v3bw -started_filepath = ${sbws_home}/started_at +state_fname = ${sbws_home}/state.dat log_dname = ${sbws_home}/log [destinations] diff --git a/sbws/core/scanner.py b/sbws/core/scanner.py index 5bcf276..5e98e99 100644 --- a/sbws/core/scanner.py +++ b/sbws/core/scanner.py @@ -7,10 +7,8 @@ from ..lib.resultdump import ResultErrorStream from ..lib.relaylist import RelayList from ..lib.relayprioritizer import RelayPrioritizer from ..lib.destination import DestinationList -from ..util.filelock import FileLock from ..util.timestamp import now_isodt_str -# from ..util.simpleauth import authenticate_to_server -# from ..util.sockio import (make_socket, close_socket) +from ..util.state import State from sbws.globals import (fail_hard, is_initted) import sbws.util.stem as stem_utils import sbws.util.requests as requests_utils @@ -316,22 +314,7 @@ def result_putter_error(target): return closure -def write_start_ts(conf): - """Write ISO formated timestamp which represents the date and time - when scanner started. - - :param ConfigParser conf: configuration - """ - generator_started = now_isodt_str() - log.info('Scanner started at {}'.format(generator_started)) - filepath = conf['paths']['started_filepath'] - with FileLock(filepath): - with open(filepath, 'w') as fd: - fd.write(generator_started) - - def run_speedtest(args, conf): - write_start_ts(conf) controller, _ = stem_utils.init_controller( path=conf['tor']['control_socket']) if not controller: @@ -398,6 +381,9 @@ def main(args, conf): os.makedirs(conf['paths']['datadir'], exist_ok=True) + state = State(conf['paths']['state_fname']) + state['scanner_started'] = now_isodt_str() + try: run_speedtest(args, conf) except KeyboardInterrupt as e: diff --git a/sbws/lib/v3bwfile.py b/sbws/lib/v3bwfile.py index c1dc890..2784c2f 100644 --- a/sbws/lib/v3bwfile.py +++ b/sbws/lib/v3bwfile.py @@ -9,8 +9,9 @@ from statistics import median from sbws import __version__ from sbws.globals import SPEC_VERSION, BW_LINE_SIZE from sbws.lib.resultdump import ResultSuccess, _ResultType -from sbws.util.filelock import FileLock, DirectoryLock +from sbws.util.filelock import DirectoryLock from sbws.util.timestamp import now_isodt_str, unixts_to_isodt_str +from sbws.util.state import State log = logging.getLogger(__name__) @@ -66,27 +67,6 @@ def warn_if_not_accurate_enough(bw_lines, scale_constant): 'allowed', (1 - accuracy_ratio) * 100, margin * 100) -def read_started_ts(conf): - """Read ISO formated timestamp which represents the date and time - when scanner started. - - :param ConfigParser conf: configuration - :returns: str, ISO formated timestamp - """ - try: - filepath = conf['paths']['started_filepath'] - except TypeError: - return None - try: - with FileLock(filepath): - with open(filepath, 'r') as fd: - generator_started = fd.read() - except FileNotFoundError as e: - log.warn('File %s not found.%s', filepath, e) - return None - return generator_started - - def num_results_of_type(results, type_str): return len([r for r in results if r.type == type_str]) @@ -210,7 +190,11 @@ class V3BWHeader(object): @staticmethod def generator_started_from_file(conf): - return read_started_ts(conf) + state = State(conf['paths']['state_fname']) + if 'scanner_started' in state: + return state['scanner_started'] + else: + return None @staticmethod def latest_bandwidth_from_results(results): diff --git a/sbws/util/config.py b/sbws/util/config.py index bfd828c..6f37bb8 100644 --- a/sbws/util/config.py +++ b/sbws/util/config.py @@ -207,8 +207,8 @@ def _validate_paths(conf): sec = 'paths' err_tmpl = Template('$sec/$key ($val): $e') unvalidated_keys = [ - 'datadir', 'sbws_home', 'v3bw_fname', 'v3bw_dname', - 'started_filepath', 'log_dname'] + 'datadir', 'sbws_home', 'v3bw_fname', 'v3bw_dname', 'state_fname', + 'log_dname'] all_valid_keys = unvalidated_keys allow_missing = ['sbws_home'] errors.extend(_validate_section_keys(conf, sec, all_valid_keys, err_tmpl,

1 0

[sbws/master] Some pythons need this tmpdir to be explicitly turned into a str
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit e00e531fef52734bd12a45ac7dde42e804295d5c Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Mon Jul 16 15:43:01 2018 -0400 Some pythons need this tmpdir to be explicitly turned into a str --- tests/unit/util/test_state.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit/util/test_state.py b/tests/unit/util/test_state.py index 42a6a8e..5e140fe 100644 --- a/tests/unit/util/test_state.py +++ b/tests/unit/util/test_state.py @@ -4,7 +4,7 @@ import os def test_state_set_allowed_key_types(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) attempt_keys = ('k') for key in attempt_keys: state[key] = 4 @@ -12,7 +12,7 @@ def test_state_set_allowed_key_types(tmpdir): def test_state_set_bad_key_types(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) attempt_keys = (15983, None, True, -1.2, [], {}, set()) for key in attempt_keys: try: @@ -32,7 +32,7 @@ def test_state_set_bad_key_types(tmpdir): def test_state_set_allowed_value_types(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) attempt_vals = (15983, None, True, -1.2, 'loooooool') for val in attempt_vals: state['foo'] = val @@ -40,7 +40,7 @@ def test_state_set_allowed_value_types(tmpdir): def test_state_set_bad_value_types(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) attempt_vals = ([], {}, set()) for val in attempt_vals: try: @@ -53,7 +53,7 @@ def test_state_set_bad_value_types(tmpdir): def test_state_del(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) d = {'a': 1, 'b': 2, 'c': 3, 'd': 4} for key in d: state[key] = d[key] @@ -83,7 +83,7 @@ def test_state_del(tmpdir): def test_state_get_len(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) d = {'a': 1, 'b': 2, 'c': 3, 'd': 4} for key in d: state[key] = d[key] @@ -101,7 +101,7 @@ def test_state_get_len(tmpdir): def test_state_contains(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) d = {'a': 1, 'b': 2, 'c': 3, 'd': 4} for key in d: state[key] = d[key] @@ -110,7 +110,7 @@ def test_state_contains(tmpdir): def test_state_iter(tmpdir): - state = State(os.path.join(tmpdir, 'statefoo')) + state = State(os.path.join(str(tmpdir), 'statefoo')) for key in state: pass d = {'a': 1, 'b': 2, 'c': 3, 'd': 4}

1 0

[sbws/master] Fix log file rotation
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit 99e3ee0d31da5ee993ef12fa96f7f5d7e960bdb3 Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Thu Jul 19 13:41:17 2018 -0400 Fix log file rotation --- CHANGELOG.md | 4 ++++ sbws/util/config.py | 8 +++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddd7f21..604811e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Document at which times should v3bw files be generated (#26740) - Remove test data v3bw file and generate it from the same test. (#26736) +### Fixed + +- Log files weren't rotating. Now they are. (#26881) + ## [0.6.0] - 2018-07-11 **Important changes**: diff --git a/sbws/util/config.py b/sbws/util/config.py index bfd828c..f7389ef 100644 --- a/sbws/util/config.py +++ b/sbws/util/config.py @@ -125,12 +125,14 @@ def configure_logging(args, conf): dname = conf['paths']['log_dname'] os.makedirs(dname, exist_ok=True) fname = os.path.join(dname, '{}.log'.format(args.command or 'sbws')) - # The second argument is the number of backups to keep, and the third - # is the maximum file size (in bytes) each log file should be. + # The second argument is the file mode, and it should be left alone + mode = 'a' + # The third is the maximum file size (in bytes) each log file should be max_bytes = conf.getint('logging', 'to_file_max_bytes') + # And the forth is the number of backups to keep num_backups = conf.getint('logging', 'to_file_num_backups') # Now store those things as a string in the config. So dumb. - conf['handler_to_file']['args'] = str((fname, num_backups, max_bytes)) + conf['handler_to_file']['args'] = str((fname, mode, max_bytes, num_backups)) # Set some stuff that needs config parser's interpolation conf['formatter_to_file']['format'] = conf['logging']['to_file_format'] conf['formatter_to_stdout']['format'] = conf['logging']['to_stdout_format']

1 0

[sbws/master] Document the state.dat file a little bit
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit 2df82656c60e0570c270720e305727512e6962c4 Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Mon Jul 16 15:08:55 2018 -0400 Document the state.dat file a little bit --- README.md | 2 ++ docs/source/index.rst | 1 + docs/source/sbws.util.rst | 8 ++++++++ docs/source/state.rst | 20 ++++++++++++++++++++ 4 files changed, 31 insertions(+) diff --git a/README.md b/README.md index 74da116..0fa6118 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,8 @@ In this directory you will find - `datadir` Once your sbws scanner has started gathering results, it will dump them into this directory. Other sbws commands (such as generate and stats) read results from the files in this directory. +- `state.dat` A file for storing state needed between sbws commands. See its + documentation for more information. ## Running tests diff --git a/docs/source/index.rst b/docs/source/index.rst index 43eb373..7f3a993 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -25,6 +25,7 @@ Contents faq glossary diagrams + state sbws Proposals diff --git a/docs/source/sbws.util.rst b/docs/source/sbws.util.rst index dd62d26..a1b3840 100644 --- a/docs/source/sbws.util.rst +++ b/docs/source/sbws.util.rst @@ -28,6 +28,14 @@ sbws.util.parser module :undoc-members: :show-inheritance: +sbws.util.state module +~~~~~~~~~~~~~~~~~~~~~ + +.. automodule:: sbws.util.state + :members: + :undoc-members: + :show-inheritance: + sbws.util.stem module ~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/source/state.rst b/docs/source/state.rst new file mode 100644 index 0000000..b61b69e --- /dev/null +++ b/docs/source/state.rst @@ -0,0 +1,20 @@ +The ``state.dat`` file +====================== + +This file contains state that multiple sbws commands may want access to and +that needs to persist across processes. Both read and write access to this file +is wrapped in the ``State`` class, allowing for safe concurrent access: the +file is locked before reading or writing, and (for now) only simple data types +are allowed so we can be sure to update the state file on disk every time the +state is modified in memory. + +At the time of writing, the following fields can exist in the state file. + +``scanner_started`` +------------------- + +The last time ``sbws scanner`` was started. + +- **Producer**: ``sbws scanner``, once at startup. + +- **Consumer**: ``sbws generate``, once each time it is ran.

1 0

[sbws/master] Fix line length
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit 53a1bf8b0c56501d9ba4037d3bc1548573ce4dd9 Author: Matt Traudt <sirmatt(a)ksu.edu> Date: Thu Jul 19 13:55:09 2018 -0400 Fix line length --- sbws/util/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sbws/util/config.py b/sbws/util/config.py index f7389ef..94b5d09 100644 --- a/sbws/util/config.py +++ b/sbws/util/config.py @@ -132,7 +132,8 @@ def configure_logging(args, conf): # And the forth is the number of backups to keep num_backups = conf.getint('logging', 'to_file_num_backups') # Now store those things as a string in the config. So dumb. - conf['handler_to_file']['args'] = str((fname, mode, max_bytes, num_backups)) + conf['handler_to_file']['args'] = \ + str((fname, mode, max_bytes, num_backups)) # Set some stuff that needs config parser's interpolation conf['formatter_to_file']['format'] = conf['logging']['to_file_format'] conf['formatter_to_stdout']['format'] = conf['logging']['to_stdout_format']

1 0

[sbws/master] Stop deleting the latest.v3bw symlink. Instead, do an atomic rename.
by pastly＠torproject.org 09 Aug '18

09 Aug '18

commit 2b111356fb813379ab0b4a0dc705706766788ad3 Author: teor <teor2345(a)gmail.com> Date: Wed Jul 18 11:12:34 2018 +1000 Stop deleting the latest.v3bw symlink. Instead, do an atomic rename. Also: * document the location of latest.v3bw * document the atomic creation of latest.v3bw * explain how to atomically create latest.v3bw when transferring it to another host Closes tor trac 26740. --- CHANGELOG.md | 6 +++++- sbws/config.default.ini | 2 ++ sbws/core/generate.py | 13 +++++++------ sbws/lib/v3bwfile.py | 19 +++++++++++++------ 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 604811e..48a7d37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Log line on start up with sbws version, platform info, and library versions (trac#26751) +### Fixed + +- Stop deleting the latest.v3bw symlink. Instead, do an atomic rename. + (#26740) + ### Changed -- Document at which times should v3bw files be generated (#26740) - Remove test data v3bw file and generate it from the same test. (#26736) ### Fixed diff --git a/sbws/config.default.ini b/sbws/config.default.ini index b21ba0b..09053a3 100644 --- a/sbws/config.default.ini +++ b/sbws/config.default.ini @@ -1,6 +1,8 @@ [paths] datadir = ${sbws_home}/datadir v3bw_dname = ${sbws_home}/v3bw +# The latest bandwidth file is atomically symlinked to +# V3BandwidthsFile ${v3bw_dname}/latest.v3bw v3bw_fname = ${v3bw_dname}/{}.v3bw started_filepath = ${sbws_home}/started_at log_dname = ${sbws_home}/log diff --git a/sbws/core/generate.py b/sbws/core/generate.py index f952fbc..481529c 100644 --- a/sbws/core/generate.py +++ b/sbws/core/generate.py @@ -12,12 +12,13 @@ log = logging.getLogger(__name__) def gen_parser(sub): d = 'Generate a v3bw file based on recent results. A v3bw file is the '\ 'file Tor directory authorities want to read and base their '\ - 'bandwidth votes on.' \ - 'This file should be generated every hour at any minute, except ' \ - 'between 45 and 55 minutes past the hour because Tor read this file '\ - ' at minute 50 and except ' \ - 'between 15 and 25 minutes past the hour because Tor read this file '\ - ' at minute 20 during a consensus failure.' + 'bandwidth votes on. '\ + 'To avoid inconsistent reads, configure tor with '\ + '"V3BandwidthsFile /path/to/latest.v3bw". '\ + '(latest.v3bw is an atomically created symlink in the same '\ + 'directory as output.) '\ + 'If the file is transferred to another host, it should be written to '\ + 'a temporary path, then renamed to the V3BandwidthsFile path.' p = sub.add_parser('generate', description=d, formatter_class=ArgumentDefaultsHelpFormatter) p.add_argument('--output', default=None, type=str, diff --git a/sbws/lib/v3bwfile.py b/sbws/lib/v3bwfile.py index c1dc890..1899e3b 100644 --- a/sbws/lib/v3bwfile.py +++ b/sbws/lib/v3bwfile.py @@ -413,17 +413,24 @@ class V3BWFile(object): log.info("Writing to stdout is not supported.") return log.info('Writing v3bw file to %s', output) + # To avoid inconsistent reads, the bandwidth data is written to an + # archive path, then atomically symlinked to 'latest.v3bw' out_dir = os.path.dirname(output) out_link = os.path.join(out_dir, 'latest.v3bw') - if os.path.exists(out_link): - log.debug('Deleting existing symlink before creating a new one.') - os.remove(out_link) + out_link_tmp = out_link + '.tmp' with DirectoryLock(out_dir): with open(output, 'wt') as fd: fd.write(str(self.header)) for line in self.bw_lines: fd.write(str(line)) output_basename = os.path.basename(output) - log.debug('Creating symlink from {} to {}.' - .format(output_basename, out_link)) - os.symlink(output_basename, out_link) + # To atomically symlink a file, we need to create a temporary link, + # then rename it to the final link name. (POSIX guarantees that + # rename is atomic.) + log.debug('Creating symlink {} -> {}.' + .format(out_link_tmp, output_basename)) + os.symlink(output_basename, out_link_tmp) + log.debug('Renaming symlink {} -> {} to {} -> {}.' + .format(out_link_tmp, output_basename, + out_link, output_basename)) + os.rename(out_link_tmp, out_link)

1 0