Ubuntu One Client

Merge lp:~verterok/ubuntuone-client/fix-848224-1-6 into lp:ubuntuone-client/stable-1-6

  1. fix-848224-1-6
  2. Merge into stable-1-6
Proposed by Guillermo Gonzalez
Status: Merged
Approved by: dobey
Approved revision: 969
Merged at revision: 971
Proposed branch: lp:~verterok/ubuntuone-client/fix-848224-1-6
Merge into: lp:ubuntuone-client/stable-1-6
Diff against target: 107 lines (+45/-11)
2 files modified
tests/syncdaemon/test_tritcask.py (+30/-0)
ubuntuone/syncdaemon/tritcask.py (+15/-11)
To merge this branch: bzr merge lp:~verterok/ubuntuone-client/fix-848224-1-6
Reviewer Review Type Date Requested Status
Guillermo Gonzalez Approve
Facundo Batista (community) Approve
Review via email: mp+75406@code.launchpad.net

Commit message

fix Tritcask.should_merge method to support missing stats for a data file.

Description of the change

This branch fix Tritcask.should_merge method to support missing stats for a data file, e.g: the file is 100% deletes (tombstones) there are no stats for it.

To post a comment you must log in.
Revision history for this message
Facundo Batista (facundo) :
review: Approve
Revision history for this message
Guillermo Gonzalez (verterok) wrote :

Approving with one review

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/syncdaemon/test_tritcask.py'
2--- tests/syncdaemon/test_tritcask.py 2011-04-05 16:50:31 +0000
3+++ tests/syncdaemon/test_tritcask.py 2011-09-15 19:44:38 +0000
4@@ -1061,6 +1061,29 @@
5 self.assertTrue(db.should_merge(db._immutable))
6 db.shutdown()
7
8+ def test_should_merge_no_stats(self):
9+ """Test should_merge method without stats for a data file."""
10+ db = Tritcask(self.base_dir, max_immutable_files=10)
11+ # add some data
12+ for j in range(10):
13+ db.put(j, 'foo%d' % (j,), 'bar%s' % (j,))
14+ # rotate the file to create a immutable with all the live rows
15+ db.rotate()
16+ # delete everything
17+ for j in range(10):
18+ db.delete(j, 'foo%d' % (j,))
19+ # rotate the file to create an immutable with all the tombstones
20+ fid = db.live_file.file_id
21+ db.rotate()
22+ db.shutdown()
23+ # start with auto_merge=False
24+ db = Tritcask(self.base_dir, auto_merge=False)
25+ # check that we don't have any stats for the file with the tombstones
26+ self.assertRaises(KeyError, db._keydir.get_stats, fid)
27+ # check the should_merge works as expected
28+ self.assertTrue(db.should_merge(db._immutable))
29+ db.shutdown()
30+
31 def test__rotate_and_not_merge(self):
32 """Test _rotate_and_merge method."""
33 db = Tritcask(self.base_dir)
34@@ -1491,6 +1514,7 @@
35 self.assertTrue(live_id > imm_id, "%d <= %d" % (live_id, imm_id))
36 db.shutdown()
37
38+
39 class KeydirStatsTests(BaseTestCase):
40 """Tests for the Keydir stats handling."""
41
42@@ -1584,6 +1608,12 @@
43 self.assertTrue(keydir._stats[file_id] is not \
44 keydir.get_stats(file_id))
45
46+ def test_get_stats_missing(self):
47+ """Test get_stats with a missing file_id."""
48+ keydir = Keydir()
49+ file_id = DataFile._get_next_file_id()
50+ self.assertRaises(KeyError, keydir.get_stats, file_id)
51+
52
53 class TritcaskShelfTests(BaseTestCase):
54 """Tests for TritcaskShelf."""
55
56=== modified file 'ubuntuone/syncdaemon/tritcask.py'
57--- ubuntuone/syncdaemon/tritcask.py 2011-04-05 16:50:31 +0000
58+++ ubuntuone/syncdaemon/tritcask.py 2011-09-15 19:44:38 +0000
59@@ -513,7 +513,6 @@
60 return self._stats[file_id].copy()
61
62
63-
64 class Tritcask(object):
65 """Implementation of a bitcask-like (row_type,key)/value store.
66
67@@ -604,25 +603,30 @@
68 # now check the default rotation policy
69 try:
70 live_file_stats = self._keydir.get_stats(self.live_file.file_id)
71+ except KeyError:
72+ # no info for the live file
73+ return False
74+ else:
75 return (live_file_stats['live_bytes'] / self.live_file.size) \
76 < self.dead_bytes_threshold
77- except KeyError:
78- # no info for the live file
79- return False
80
81 def should_merge(self, immutable_files):
82 """Check if the immutable_files should be merged."""
83 if not immutable_files:
84 return False
85- file_ids = sorted(immutable_files.keys())
86- live_bytes = sum([self._keydir.get_stats(file_id)['live_bytes'] \
87- for file_id in file_ids])
88+ live_bytes = 0
89+ for file_id in sorted(immutable_files.keys()):
90+ try:
91+ stats = self._keydir.get_stats(file_id)
92+ except KeyError:
93+ # no stats for this data file
94+ pass
95+ else:
96+ live_bytes += stats['live_bytes']
97 total_bytes = sum([f.size for f in immutable_files.values()])
98 merge_dead = (live_bytes / total_bytes) < self.dead_bytes_threshold
99- if merge_dead:
100- return True
101- # now check if we should merge using the max_immutable_files setting.
102- if len(self._immutable) > self.max_immutable_files:
103+ # check if we should merge using the max_immutable_files setting.
104+ if merge_dead or len(self._immutable) > self.max_immutable_files:
105 return True
106 # shouldn't merge.
107 return False

Subscribers

People subscribed via source and target branches