launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/librarian-fsync into lp:launchpad.

Commit message:
Fix the librarian to fsync new files and their parent directories.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/librarian-fsync/+merge/306099

Fix the librarian to fsync new files and their parent directories.

Previously an unclean shutdown could result in truncated or missing
files, even after the upload appeared successful and the client had
committed the database transaction. The fsyncs only ensure durability,
as we don't care about atomicity.

-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/librarian-fsync into lp:launchpad.

=== modified file 'lib/lp/services/librarianserver/storage.py'
--- lib/lp/services/librarianserver/storage.py	2015-12-27 04:00:36 +0000
+++ lib/lp/services/librarianserver/storage.py	2016-09-19 13:43:37 +0000
@@ -37,6 +37,34 @@
     ]
 
 
+def fsync_path(path, dir=False):
+    fd = os.open(path, os.O_RDONLY | (os.O_DIRECTORY if dir else 0))
+    try:
+        os.fsync(fd)
+    finally:
+        os.close(fd)
+
+
+def makedirs_fsync(name, mode=0777):
+    """makedirs_fsync(path [, mode=0777])
+
+    os.makedirs, but fsyncing on the way up to ensure durability.
+    """
+    head, tail = os.path.split(name)
+    if not tail:
+        head, tail = os.path.split(head)
+    if head and tail and not os.path.exists(head):
+        try:
+            makedirs_fsync(head, mode)
+        except OSError, e:
+            if e.errno != errno.EEXIST:
+                raise
+        if tail == os.curdir:
+            return
+    os.mkdir(name, mode)
+    fsync_path(head, dir=True)
+
+
 class DigestMismatchError(Exception):
     """The given digest doesn't match the SHA-1 digest of the file."""
 
@@ -272,12 +300,14 @@
         if os.path.exists(location):
             raise DuplicateFileIDError(fileID)
         try:
-            os.makedirs(os.path.dirname(location))
+            makedirs_fsync(os.path.dirname(location))
         except OSError as e:
             # If the directory already exists, that's ok.
             if e.errno != errno.EEXIST:
                 raise
         shutil.move(self.tmpfilepath, location)
+        fsync_path(location)
+        fsync_path(os.path.dirname(location), dir=True)
 
 
 def _sameFile(path1, path2):