commit 2b111356fb813379ab0b4a0dc705706766788ad3 Author: teor teor2345@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)