Merge lp:~david-goetz/swift/auditor_bug into lp:~hudson-openstack/swift/trunk

Proposed by David Goetz
Status: Merged
Approved by: John Dickinson
Approved revision: 350
Merged at revision: 350
Proposed branch: lp:~david-goetz/swift/auditor_bug
Merge into: lp:~hudson-openstack/swift/trunk
Diff against target: 93 lines (+47/-24)
2 files modified
swift/obj/auditor.py (+27/-24)
test/unit/obj/test_auditor.py (+20/-0)
To merge this branch: bzr merge lp:~david-goetz/swift/auditor_bug
Reviewer Review Type Date Requested Status
John Dickinson Approve
gholt (community) Approve
Review via email: mp+73449@code.launchpad.net

Description of the change

Fix for object auditor. It doesn't close files that are quarantined for certain reasons, zero byte files for one, which will cause it to eventually crash due to keeping too many files open. Thanks David Kranz for finding / reporting this!!

To post a comment you must log in.
Revision history for this message
gholt (gholt) wrote :

Your was_df stuff should have a try/finally

review: Needs Fixing
lp:~david-goetz/swift/auditor_bug updated
350. By David Goetz

fix for unit test

Revision history for this message
David Goetz (david-goetz) wrote :

added the try finally.

Revision history for this message
gholt (gholt) wrote :

:)

review: Approve
Revision history for this message
John Dickinson (notmyname) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'swift/obj/auditor.py'
2--- swift/obj/auditor.py 2011-05-23 20:20:09 +0000
3+++ swift/obj/auditor.py 2011-08-31 14:28:32 +0000
4@@ -134,31 +134,34 @@
5 df = object_server.DiskFile(self.devices, device, partition,
6 account, container, obj, self.logger,
7 keep_data_fp=True)
8- if df.data_file is None:
9- # file is deleted, we found the tombstone
10- return
11 try:
12- obj_size = df.get_data_file_size()
13- except DiskFileError, e:
14- raise AuditException(str(e))
15- except DiskFileNotExist:
16- return
17- if self.zero_byte_only_at_fps and obj_size:
18- self.passes += 1
19- return
20- for chunk in df:
21- self.bytes_running_time = ratelimit_sleep(
22- self.bytes_running_time, self.max_bytes_per_second,
23- incr_by=len(chunk))
24- self.bytes_processed += len(chunk)
25- self.total_bytes_processed += len(chunk)
26- df.close()
27- if df.quarantined_dir:
28- self.quarantines += 1
29- self.logger.error(
30- _("ERROR Object %(path)s failed audit and will be "
31- "quarantined: ETag and file's md5 do not match"),
32- {'path': path})
33+ if df.data_file is None:
34+ # file is deleted, we found the tombstone
35+ return
36+ try:
37+ obj_size = df.get_data_file_size()
38+ except DiskFileError, e:
39+ raise AuditException(str(e))
40+ except DiskFileNotExist:
41+ return
42+ if self.zero_byte_only_at_fps and obj_size:
43+ self.passes += 1
44+ return
45+ for chunk in df:
46+ self.bytes_running_time = ratelimit_sleep(
47+ self.bytes_running_time, self.max_bytes_per_second,
48+ incr_by=len(chunk))
49+ self.bytes_processed += len(chunk)
50+ self.total_bytes_processed += len(chunk)
51+ df.close()
52+ if df.quarantined_dir:
53+ self.quarantines += 1
54+ self.logger.error(
55+ _("ERROR Object %(path)s failed audit and will be "
56+ "quarantined: ETag and file's md5 do not match"),
57+ {'path': path})
58+ finally:
59+ df.close(verify_file=False)
60 except AuditException, err:
61 self.quarantines += 1
62 self.logger.error(_('ERROR Object %(obj)s failed audit and will '
63
64=== modified file 'test/unit/obj/test_auditor.py'
65--- test/unit/obj/test_auditor.py 2011-03-16 14:55:07 +0000
66+++ test/unit/obj/test_auditor.py 2011-08-31 14:28:32 +0000
67@@ -315,6 +315,26 @@
68 my_auditor._sleep()
69 self.assertEquals(round(time.time() - start, 2), 0.01)
70
71+ def test_object_run_fast_track_zero_check_closed(self):
72+ rat = [False]
73+
74+ class FakeFile(DiskFile):
75+
76+ def close(self, verify_file=True):
77+ rat[0] = True
78+ DiskFile.close(self, verify_file=verify_file)
79+ self.setup_bad_zero_byte()
80+ was_df = object_server.DiskFile
81+ try:
82+ object_server.DiskFile = FakeFile
83+ self.auditor.run_once(zero_byte_fps=50)
84+ quarantine_path = os.path.join(self.devices,
85+ 'sda', 'quarantined', 'objects')
86+ self.assertTrue(os.path.isdir(quarantine_path))
87+ self.assertTrue(rat[0])
88+ finally:
89+ object_server.DiskFile = was_df
90+
91 def test_run_forever(self):
92
93 class StopForever(Exception):