From 2c0bbaf05c06968cc0376f34acbfb09e72603d47 Mon Sep 17 00:00:00 2001 From: Peter Portante Date: Sun, 24 Nov 2013 23:29:53 -0500 Subject: [PATCH] Protect against hash cleanup errors on PUTs In http://launchpad.net/bugs/1254405 an exception occurs when finalizing the PUT of an object, but it is obscured by the thread pool code, so we don't see the actual line where it originated. However, there are two possible functions where this exception could originate: 1. renamer() 2. hash_cleanup_listdir() If this is an error in renamer(), there is some other waky problem where the temporary file has been removed. It is likely that this is a problem where a file name from os.listdir ends up disappearing (but even this is a rare occurence). Regardless, it is not clear that we really want an error from hash_cleanup_listdir() from affecting the return result of the PUT. To that end, we squelsh OSErrors from hash_cleanup_listdir() for now. One might argue for all errors, but since os.unlink() and os.listdir() raise OSError today, that is probably sufficient. Even that we use "Closes-Bug" below, it is not clear it can even be determined that this closes that bug report. Change-Id: I2f55df835c387e4d17cffda74c04c9994aebbe1f Closes-Bug: 1254405 Signed-off-by: Peter Portante --- swift/obj/diskfile.py | 5 ++++- test/unit/obj/test_diskfile.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py index f8c1d10037..6ee77974e1 100644 --- a/swift/obj/diskfile.py +++ b/swift/obj/diskfile.py @@ -662,7 +662,10 @@ class DiskFileWriter(object): # After the rename completes, this object will be available for other # requests to reference. renamer(self._tmppath, target_path) - hash_cleanup_listdir(self._datadir) + try: + hash_cleanup_listdir(self._datadir) + except OSError: + logging.exception(_('Problem cleaning up %s'), self._datadir) def put(self, metadata): """ diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index e038636ed4..6b39aba5cf 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -1590,6 +1590,24 @@ class TestDiskFile(unittest.TestCase): with df.open(): self.assertEqual(df.timestamp, '1383181759.12345') + def test_error_in_hashdir_cleanup_listdir(self): + + def mock_hcl(*args, **kwargs): + raise OSError() + + df = self._get_open_disk_file() + ts = time() + with mock.patch("swift.obj.diskfile.hash_cleanup_listdir", + mock_hcl): + try: + df.delete(ts) + except OSError: + self.fail("OSError raised when it should have been swallowed") + exp_name = '%s.ts' % str(normalize_timestamp(ts)) + dl = os.listdir(df._datadir) + self.assertEquals(len(dl), 2) + self.assertTrue(exp_name in set(dl)) + if __name__ == '__main__': unittest.main()