Launchpad itself

Merge lp:~wgrant/launchpad/librarian-fsync into lp:launchpad

  1. librarian-fsync
  2. Merge into devel
Proposed by William Grant
Status: Merged
Merged at revision: 18196
Proposed branch: lp:~wgrant/launchpad/librarian-fsync
Merge into: lp:launchpad
Diff against target: 54 lines (+31/-1)
1 file modified
lib/lp/services/librarianserver/storage.py (+31/-1)
To merge this branch: bzr merge lp:~wgrant/launchpad/librarian-fsync
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+306099@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/librarianserver/storage.py'
2--- lib/lp/services/librarianserver/storage.py 2015-12-27 04:00:36 +0000
3+++ lib/lp/services/librarianserver/storage.py 2016-09-19 13:54:29 +0000
4@@ -37,6 +37,34 @@
5 ]
6
7
8+def fsync_path(path, dir=False):
9+ fd = os.open(path, os.O_RDONLY | (os.O_DIRECTORY if dir else 0))
10+ try:
11+ os.fsync(fd)
12+ finally:
13+ os.close(fd)
14+
15+
16+def makedirs_fsync(name, mode=0777):
17+ """makedirs_fsync(path [, mode=0777])
18+
19+ os.makedirs, but fsyncing on the way up to ensure durability.
20+ """
21+ head, tail = os.path.split(name)
22+ if not tail:
23+ head, tail = os.path.split(head)
24+ if head and tail and not os.path.exists(head):
25+ try:
26+ makedirs_fsync(head, mode)
27+ except OSError as e:
28+ if e.errno != errno.EEXIST:
29+ raise
30+ if tail == os.curdir:
31+ return
32+ os.mkdir(name, mode)
33+ fsync_path(head, dir=True)
34+
35+
36 class DigestMismatchError(Exception):
37 """The given digest doesn't match the SHA-1 digest of the file."""
38
39@@ -272,12 +300,14 @@
40 if os.path.exists(location):
41 raise DuplicateFileIDError(fileID)
42 try:
43- os.makedirs(os.path.dirname(location))
44+ makedirs_fsync(os.path.dirname(location))
45 except OSError as e:
46 # If the directory already exists, that's ok.
47 if e.errno != errno.EEXIST:
48 raise
49 shutil.move(self.tmpfilepath, location)
50+ fsync_path(location)
51+ fsync_path(os.path.dirname(location), dir=True)
52
53
54 def _sameFile(path1, path2):