Merge lp:~verterok/ubuntuone-client/tritcask-fix-825366-stable-2-0 into lp:ubuntuone-client/stable-2-0

[Guillermo Gonzalez]

Make sure we never use or create a hint file with size 0.

Make sure we never use or create a hint file with size 0.

[Curtis Caravone (caravone)]

review: Approve

[Roberto Alsina (ralsina)]

+1

review: Approve

=== modified file 'tests/syncdaemon/test_tritcask.py'
--- tests/syncdaemon/test_tritcask.py	2011-09-26 20:22:12 +0000
+++ tests/syncdaemon/test_tritcask.py	2011-12-23 04:16:28 +0000
@@ -140,6 +140,16 @@
         new_file.fd.flush()
         self.assertEqual(len('foo'), new_file.size)
 
+    def test_has_hint(self):
+        """Test that has_hint works as expetced."""
+        new_file = self.file_class(self.base_dir)
+        self.assertFalse(new_file.has_hint)
+
+    def test_hint_size(self):
+        """Test that hint_size work as expected."""
+        new_file = self.file_class(self.base_dir)
+        self.assertEqual(new_file.hint_size, 0)
+
     def test__open(self):
         """Test the _open private method."""
         new_file = self.file_class(self.base_dir)
@@ -523,6 +533,26 @@
         self.assertEqual(len('bar')+len('foo')+header_size+crc32_size,
                          data_file.size)
 
+    def test_has_hint(self):
+        """Test that has_hint works as expetced."""
+        rw_file = DataFile(self.base_dir)
+        tstamp, value_pos, value_sz = rw_file.write(0, 'foo', 'bar')
+        data_file = rw_file.make_immutable()
+        self.assertFalse(data_file.has_hint)
+        data_file.get_hint_file().close()
+        self.assertTrue(data_file.has_hint)
+
+    def test_hint_size(self):
+        """Test that hint_size work as expected."""
+        rw_file = DataFile(self.base_dir)
+        tstamp, value_pos, value_sz = rw_file.write(0, 'foo', 'bar')
+        data_file = rw_file.make_immutable()
+        self.assertEqual(data_file.hint_size, 0)
+        hint_file = data_file.get_hint_file()
+        hint_file.fd.write("some data")
+        hint_file.close()
+        self.assertEqual(data_file.hint_size, len("some data"))
+
 
 class DeadDataFileTest(ImmutableDataFileTest):
     """Tests for DeadDataFile."""
@@ -617,13 +647,22 @@
         self.assertEqual('w+b', hint_file.fd.mode)
 
     def test_init_existing(self):
-        """Test initialization."""
+        """Test initialization with existing file."""
         path = os.path.join(self.base_dir, 'test_hint_existing')
         hint_file = HintFile(path)
+        hint_file.fd.write("some data")
         hint_file.close()
         hint_file = HintFile(path)
         self.assertEqual('rb', hint_file.fd.mode)
 
+    def test_init_existing_empty(self):
+        """Test initialization with existing file."""
+        path = os.path.join(self.base_dir, 'test_hint_existing')
+        hint_file = HintFile(path)
+        hint_file.close()
+        hint_file = HintFile(path)
+        self.assertEqual('w+b', hint_file.fd.mode)
+
     def test_close(self):
         """Test for the close method."""
         path = os.path.join(self.base_dir, 'test_hint_close')
@@ -1032,6 +1071,27 @@
             self.assertEqual(6, len(hint_entry))
             self.assertTrue(db.get(hint_entry[2], hint_entry[-1]) is not None)
 
+    def test_build_keydir_with_empty_hint(self):
+        """Test _build_keydir using a hint file."""
+        db = Tritcask(self.base_dir)
+        # create some stuff.
+        for i in range(10):
+            db.put(i, 'foo%d' % (i,), 'bar%s' % (i,))
+        # make the data file inactive and generate the hint.
+        db.live_file.make_immutable()
+        old_keydir = db._keydir
+        db.shutdown()
+        db = Tritcask(self.base_dir)
+        hint_filename = db._immutable.values()[0].hint_filename
+        # truncate the hint file
+        open(hint_filename, 'w').close()
+        db.shutdown()
+        # open the tritcask and make sure it regenerates the hint file
+        db = Tritcask(self.base_dir)
+        self.addCleanup(db.shutdown)
+        self.assertTrue(os.path.exists(hint_filename), os.listdir(db.base_path))
+        self.assertEqual(old_keydir, db._keydir)
+
     def test_build_keydir(self):
         """Test _build_keydir method."""
         db = Tritcask(self.base_dir)

=== modified file 'ubuntuone/syncdaemon/tritcask.py'
--- ubuntuone/syncdaemon/tritcask.py	2011-09-26 20:22:12 +0000
+++ ubuntuone/syncdaemon/tritcask.py	2011-12-23 04:16:28 +0000
@@ -202,6 +202,13 @@
         """Return true if there is a hint file on-disk."""
         return os.path.exists(self.hint_filename)
 
+    @property
+    def hint_size(self):
+        """Return the hint file size."""
+        if self.has_hint:
+            return os.stat(self.hint_filename).st_size
+        return 0
+
     def exists(self):
         return os.path.exists(self.filename)
 
@@ -433,8 +440,8 @@
         """Create the instance."""
         self.path = path
         self.tempfile = None
-        if os.path.exists(self.path):
-            # if it's there open only for read
+        if os.path.exists(self.path) and os.stat(self.path).st_size > 0:
+            # if it's there and size > 0, open only for read
             self.fd = open(self.path, 'rb')
         else:
             # this is a new hint file, lets create it as a tempfile.
@@ -755,8 +762,8 @@
         # sort the files by name, in order to load from older -> newest
         for data_file in sorted(self._immutable.values(),
                                 key=attrgetter('filename')):
-            if data_file.has_hint:
-                # load directly from the hint file.
+            if data_file.has_hint and data_file.hint_size > 0:
+                # load directly from the hint file, only if the size > 0
                 self._load_from_hint(data_file)
             elif data_file.exists() and data_file.size > 0:
                 self._load_from_data(data_file)
@@ -808,7 +815,8 @@
                 if build_hint:
                     hint_entry = HintEntry.from_tritcask_entry(entry)
                     hint_idx[hint_entry.key] = hint_entry
-        if build_hint:
+        if build_hint and hint_idx:
+            # only build the hint file if hint_idx contains data
             with data_file.get_hint_file() as hint_file:
                 for key, hint_entry in hint_idx.iteritems():
                     hint_file.write(hint_entry)