[tor-commits] [sbws/master] Stop deleting the latest.v3bw symlink. Instead, do an atomic rename.

[pastly at torproject.org]

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