diff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py index a54545de10..7c0a9483a1 100644 --- a/swift/obj/diskfile.py +++ b/swift/obj/diskfile.py @@ -52,7 +52,7 @@ from swift.common.constraints import check_mount from swift.common.utils import mkdirs, normalize_timestamp, \ storage_directory, hash_path, renamer, fallocate, fsync, \ fdatasync, drop_buffer_cache, ThreadPool, lock_path, write_pickle, \ - config_true_value, listdir, split_path, ismount + config_true_value, listdir, split_path, ismount, remove_file from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \ DiskFileCollision, DiskFileNoSpace, DiskFileDeviceUnavailable, \ DiskFileDeleted, DiskFileError, DiskFileNotOpen, PathNotDir, \ @@ -185,13 +185,13 @@ def hash_cleanup_listdir(hsh_path, reclaim_age=ONE_WEEK): :param reclaim_age: age in seconds at which to remove tombstones :returns: list of files remaining in the directory, reverse sorted """ - files = os.listdir(hsh_path) + files = listdir(hsh_path) if len(files) == 1: if files[0].endswith('.ts'): # remove tombstones older than reclaim_age ts = files[0].rsplit('.', 1)[0] if (time.time() - float(ts)) > reclaim_age: - os.unlink(join(hsh_path, files[0])) + remove_file(join(hsh_path, files[0])) files.remove(files[0]) elif files: files.sort(reverse=True) @@ -202,7 +202,7 @@ def hash_cleanup_listdir(hsh_path, reclaim_age=ONE_WEEK): or (meta_file and filename.endswith('.meta') and filename < meta_file)): - os.unlink(join(hsh_path, filename)) + remove_file(join(hsh_path, filename)) files.remove(filename) return files diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index cdbab4ee7f..d86bb9d30c 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -23,6 +23,7 @@ import mock import unittest import email import tempfile +import uuid import xattr from shutil import rmtree from time import time @@ -428,12 +429,19 @@ class TestDiskFileModuleMethods(unittest.TestCase): self.assertEquals(i[0], 3) def check_hash_cleanup_listdir(self, input_files, output_files): + orig_unlink = os.unlink file_list = list(input_files) def mock_listdir(path): return list(file_list) def mock_unlink(path): + # timestamp 1 is a special tag to pretend a file disappeared while + # working. + if '/0000000001.00000.' in path: + # Using actual os.unlink to reproduce exactly what OSError it + # raises. + orig_unlink(uuid.uuid4().hex) file_list.remove(os.path.basename(path)) with unit_mock({'os.listdir': mock_listdir, 'os.unlink': mock_unlink}): @@ -528,6 +536,26 @@ class TestDiskFileModuleMethods(unittest.TestCase): file_list = [file1] self.check_hash_cleanup_listdir(file_list, [file1]) + def test_hash_cleanup_listdir_disappeared_path(self): + # Next line listing a non-existent dir used to propagate the OSError; + # now should mute that. + self.assertEqual(diskfile.hash_cleanup_listdir(uuid.uuid4().hex), []) + + def test_hash_cleanup_listdir_disappeared_before_unlink_1(self): + # Timestamp 1 makes other test routines pretend the file disappeared + # while working. + file1 = '0000000001.00000.ts' + file_list = [file1] + self.check_hash_cleanup_listdir(file_list, []) + + def test_hash_cleanup_listdir_disappeared_before_unlink_2(self): + # Timestamp 1 makes other test routines pretend the file disappeared + # while working. + file1 = '0000000001.00000.data' + file2 = '0000000002.00000.ts' + file_list = [file1, file2] + self.check_hash_cleanup_listdir(file_list, [file2]) + class TestObjectAuditLocationGenerator(unittest.TestCase): def _make_file(self, path):