From 4d4db054d35c7e9de9ed28ac24965e8408dd9f82 Mon Sep 17 00:00:00 2001 From: Minwoo Bae Date: Mon, 18 May 2015 14:08:25 -0500 Subject: [PATCH] After the .durable has been written, fsync the directory. Added try-except statements in _finalize_durable() to fsync the directory after a successful fsync of the .durable file. Added test_commit_fsync_dir_raises_DiskFileErrors() for testing whether certain assertions hold for the change to include fsync_dir(). Some more error details have been included in the logger. Closes-Bug: #1470651 Change-Id: I4791d75ade8542678369ba0811ef39af6e955cc6 --- swift/obj/diskfile.py | 46 ++++++++++++++++++++++-------- test/unit/obj/test_diskfile.py | 52 ++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/swift/obj/diskfile.py b/swift/obj/diskfile.py index 3920315551..2e01137dc1 100644 --- a/swift/obj/diskfile.py +++ b/swift/obj/diskfile.py @@ -53,8 +53,8 @@ from swift import gettext_ as _ from swift.common.constraints import check_mount, check_dir from swift.common.request_helpers import is_sys_meta from swift.common.utils import mkdirs, Timestamp, \ - storage_directory, hash_path, renamer, fallocate, fsync, \ - fdatasync, drop_buffer_cache, ThreadPool, lock_path, write_pickle, \ + storage_directory, hash_path, renamer, fallocate, fsync, fdatasync, \ + fsync_dir, drop_buffer_cache, ThreadPool, lock_path, write_pickle, \ config_true_value, listdir, split_path, ismount, remove_file, \ get_md5_socket, F_SETPIPE_SZ from swift.common.splice import splice, tee @@ -1788,23 +1788,45 @@ class ECDiskFileWriter(DiskFileWriter): try: with open(durable_file_path, 'w') as _fp: fsync(_fp.fileno()) + try: + fsync_dir(self._datadir) + except OSError as os_err: + msg = (_('%s \nProblem fsyncing dir' + 'after writing .durable: %s') % + (os_err, self._datadir)) + exc = DiskFileError(msg) + except IOError as io_err: + if io_err.errno in (errno.ENOSPC, errno.EDQUOT): + msg = (_('%s \nNo space left on device' + 'for updates to: %s') % + (io_err, self._datadir)) + exc = DiskFileNoSpace(msg) + else: + msg = (_('%s \nProblem fsyncing dir' + 'after writing .durable: %s') % + (io_err, self._datadir)) + exc = DiskFileError(msg) + if exc: + self.manager.logger.exception(msg) + raise exc try: self.manager.hash_cleanup_listdir(self._datadir) - except OSError: + except OSError as os_err: self.manager.logger.exception( - _('Problem cleaning up %s'), self._datadir) - except OSError: - msg = (_('Problem fsyncing durable state file: %s'), - durable_file_path) + _('%s \nProblem cleaning up %s') % + (os_err, self._datadir)) + except OSError as os_err: + msg = (_('%s \nProblem fsyncing durable state file: %s') % + (os_err, durable_file_path)) exc = DiskFileError(msg) except IOError as io_err: if io_err.errno in (errno.ENOSPC, errno.EDQUOT): - msg = (_("No space left on device for %s"), - durable_file_path) - exc = DiskFileNoSpace() + msg = (_('%s \nNo space left on device for %s') % + (io_err, durable_file_path)) + exc = DiskFileNoSpace(msg) else: - msg = (_('Problem writing durable state file: %s'), - durable_file_path) + msg = (_('%s \nProblem writing durable state file: %s') % + (io_err, durable_file_path)) exc = DiskFileError(msg) if exc: self.manager.logger.exception(msg) diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index a84cafc8b4..67e01ecec8 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -3262,6 +3262,58 @@ class TestECDiskFile(DiskFileMixin, unittest.TestCase): self.assertRaises(DiskFileError, writer.commit, timestamp) + def test_commit_fsync_dir_raises_DiskFileErrors(self): + scenarios = ((errno.ENOSPC, DiskFileNoSpace), + (errno.EDQUOT, DiskFileNoSpace), + (errno.ENOTDIR, DiskFileError), + (errno.EPERM, DiskFileError)) + + # Check IOErrors from fsync_dir() is handled + for err_number, expected_exception in scenarios: + io_error = IOError() + io_error.errno = err_number + mock_open = mock.MagicMock(side_effect=io_error) + mock_io_error = mock.MagicMock(side_effect=io_error) + df = self._simple_get_diskfile(account='a', container='c', + obj='o_%s' % err_number, + policy=POLICIES.default) + timestamp = Timestamp(time()) + with df.create() as writer: + metadata = { + 'ETag': 'bogus_etag', + 'X-Timestamp': timestamp.internal, + 'Content-Length': '0', + } + writer.put(metadata) + with mock.patch('__builtin__.open', mock_open): + self.assertRaises(expected_exception, + writer.commit, + timestamp) + with mock.patch('swift.obj.diskfile.fsync_dir', mock_io_error): + self.assertRaises(expected_exception, + writer.commit, + timestamp) + dl = os.listdir(df._datadir) + self.assertEqual(2, len(dl), dl) + rmtree(df._datadir) + + # Check OSError from fsync_dir() is handled + mock_os_error = mock.MagicMock(side_effect=OSError) + df = self._simple_get_diskfile(account='a', container='c', + obj='o_fsync_dir_error') + + timestamp = Timestamp(time()) + with df.create() as writer: + metadata = { + 'ETag': 'bogus_etag', + 'X-Timestamp': timestamp.internal, + 'Content-Length': '0', + } + writer.put(metadata) + with mock.patch('swift.obj.diskfile.fsync_dir', mock_os_error): + self.assertRaises(DiskFileError, + writer.commit, timestamp) + def test_data_file_has_frag_index(self): policy = POLICIES.default for good_value in (0, '0', 2, '2', 14, '14'):