From d90d1ca90000eee0a7aa844b23a51cb5edbc7097 Mon Sep 17 00:00:00 2001 From: Thiago da Silva Date: Wed, 16 Sep 2015 00:01:38 +0000 Subject: [PATCH] open and close ioctx only once The io context handle should be open only once in the life of the application, there's no need to open and close with every I/O transaction Change-Id: I9948a71dbdcd20b4732c109ad35a400bd5962766 Signed-off-by: Thiago da Silva --- swift_ceph_backend/rados_diskfile.py | 55 +++++++++++----------------- swift_ceph_backend/rados_server.py | 3 +- tests/test_rados_diskfile.py | 16 ++------ 3 files changed, 26 insertions(+), 48 deletions(-) diff --git a/swift_ceph_backend/rados_diskfile.py b/swift_ceph_backend/rados_diskfile.py index f567c91..13b8b03 100644 --- a/swift_ceph_backend/rados_diskfile.py +++ b/swift_ceph_backend/rados_diskfile.py @@ -22,17 +22,17 @@ import time from swift.common.utils import normalize_timestamp from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \ - DiskFileCollision, DiskFileDeleted, DiskFileNotOpen, DiskFileNoSpace, \ - DiskFileError + DiskFileCollision, DiskFileNotOpen, DiskFileNoSpace, DiskFileError from swift.common.swob import multi_range_iterator from swift.obj.diskfile import METADATA_KEY class RadosFileSystem(object): - def __init__(self, ceph_conf, rados_user, rados_pool, **kwargs): + def __init__(self, ceph_conf, rados_user, rados_pool, logger, **kwargs): self._conf = ceph_conf self._user = rados_user self._pool = rados_pool + self._logger = logger self._rados = None self._ioctx = None @@ -47,21 +47,19 @@ class RadosFileSystem(object): if self._rados is None: self._rados = fs.RADOS.Rados(conffile=fs._conf, rados_id=fs._user) self._rados.connect() - return self._rados + self._ioctx = self._rados.open_ioctx(self._pool) + return self._rados, self._ioctx def _shutdown(self): if self._rados: + self._ioctx.close() self._rados.shutdown() class _radosfs(object): def __init__(self, fs, _get_rados): - self._rados = _get_rados(fs) + self._rados, self._ioctx = _get_rados(fs) self._fs = fs self._pool = fs._pool - self._ioctx = self._rados.open_ioctx(self._pool) - - def close(self): - self._ioctx.close() def del_object(self, obj): try: @@ -189,7 +187,6 @@ class DiskFileReader(object): self._obj_size = obj_size self._etag = etag self._iter_hook = iter_hook - # self._iter_etag = None self._bytes_read = 0 self._read_offset = 0 @@ -275,9 +272,8 @@ class DiskFileReader(object): def close(self): """ - Close the file. Will handle quarantining file if necessary. + handle quarantining file if necessary. """ - self._fs.close() try: if self._started_at_0 and self._read_to_eof: self._handle_close_quarantine() @@ -312,15 +308,14 @@ class DiskFile(object): This method must populate the _metadata attribute. :raises DiskFileCollision: on name mis-match with metadata - :raises DiskFileDeleted: if it does not exist, or a tombstone is - present + :raises DiskFileNotExist: if it does not exist :raises DiskFileQuarantined: if while reading metadata of the file - some data did pass cross checks + some data did not pass cross checks """ self._fs_inst = self._fs.open() self._metadata = self._fs_inst.get_metadata(self._name) if self._metadata is None: - raise DiskFileDeleted() + raise DiskFileNotExist() self._verify_data_file() self._metadata = self._metadata or {} return self @@ -331,7 +326,7 @@ class DiskFile(object): return self def __exit__(self, t, v, tb): - self._fs_inst.close() + pass def _quarantine(self, msg): self._fs_inst.quarantine(self._name) @@ -437,15 +432,11 @@ class DiskFile(object): :raises DiskFileNoSpace: if a size is specified and allocation fails """ fs_inst = None - try: - fs_inst = self._fs.open() - if size is not None: - fs_inst.create(self._name, size) + fs_inst = self._fs.open() + if size is not None: + fs_inst.create(self._name, size) - yield DiskFileWriter(fs_inst, self._name) - finally: - if fs_inst is not None: - fs_inst.close() + yield DiskFileWriter(fs_inst, self._name) def write_metadata(self, metadata): """ @@ -462,12 +453,8 @@ class DiskFile(object): :param timestamp: timestamp to compare with each file """ fs_inst = None - try: - timestamp = normalize_timestamp(timestamp) - fs_inst = self._fs.open() - md = fs_inst.get_metadata(self._name) - if md and md['X-Timestamp'] < timestamp: - fs_inst.del_object(self._name) - finally: - if fs_inst is not None: - fs_inst.close() + timestamp = normalize_timestamp(timestamp) + fs_inst = self._fs.open() + md = fs_inst.get_metadata(self._name) + if md and md['X-Timestamp'] < timestamp: + fs_inst.del_object(self._name) diff --git a/swift_ceph_backend/rados_server.py b/swift_ceph_backend/rados_server.py index a64b739..1c0fb39 100644 --- a/swift_ceph_backend/rados_server.py +++ b/swift_ceph_backend/rados_server.py @@ -39,7 +39,8 @@ class ObjectController(server.ObjectController): ceph_conf = conf.get("rados_ceph_conf", None) rados_user = conf.get("rados_user", None) rados_pool = conf.get("rados_pool", None) - self._filesystem = RadosFileSystem(ceph_conf, rados_user, rados_pool) + self._filesystem = RadosFileSystem(ceph_conf, rados_user, rados_pool, + self.logger) def get_diskfile(self, device, partition, account, container, obj, **kwargs): diff --git a/tests/test_rados_diskfile.py b/tests/test_rados_diskfile.py index 3d54b99..6e6fa56 100644 --- a/tests/test_rados_diskfile.py +++ b/tests/test_rados_diskfile.py @@ -39,9 +39,11 @@ class TestRadosDiskFile(unittest.TestCase): self.account = 'account' self.container = 'container' self.obj_name = 'myobject' + self.logger = mock.Mock() self.rdf = RadosFileSystem(self.rados_ceph_conf, self.rados_name, self.rados_pool, + self.logger, rados=self.mock_rados) self.df = self.rdf.get_diskfile(self.device, self.partition, @@ -67,9 +69,6 @@ class TestRadosDiskFile(unittest.TestCase): self.Rados.connect.assert_called_once_with() self.Rados.open_ioctx.assert_called_once_with(self.rados_pool) - def _assert_if_rados_not_closed(self): - self.ioctx.close.assert_called_once_with() - def _assert_if_rados_opened(self): assert((self.mock_rados.Rados.call_count == 0) and (self.Rados.connect.call_count == 0) and @@ -85,7 +84,7 @@ class TestRadosDiskFile(unittest.TestCase): (self.Rados.open_ioctx.call_count > 0)) assert((self.Rados.connect.call_count > 0) and (self.Rados.open_ioctx.call_count == - self.ioctx.close.call_count)) + self.Rados.connect.call_count)) def test_df_open_1(self): meta = {'name': self._obj_name(), 'Content-Length': 0} @@ -96,7 +95,6 @@ class TestRadosDiskFile(unittest.TestCase): self._assert_if_rados_not_opened() self.ioctx.get_xattr.assert_called_once_with(self._obj_name(), METADATA_KEY) - self._assert_if_rados_not_closed() def test_df_open_invalid_name(self): meta = {'name': 'invalid', 'Content-Length': 0} @@ -169,7 +167,6 @@ class TestRadosDiskFile(unittest.TestCase): assert(success) assert(ret_meta == meta) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() def test_df_read_metadata(self): meta = {'name': self._obj_name(), 'Content-Length': 0} @@ -186,7 +183,6 @@ class TestRadosDiskFile(unittest.TestCase): assert(success) assert(ret_meta == meta) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() def test_df_notopen_reader(self): success = False @@ -215,7 +211,6 @@ class TestRadosDiskFile(unittest.TestCase): finally: assert(success) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() def test_df_open_reader_2(self): meta = {'name': self._obj_name(), 'Content-Length': 0, 'ETag': ''} @@ -386,7 +381,6 @@ class TestRadosDiskFile(unittest.TestCase): pass assert(self.ioctx.trunc.call_count == 0) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() with self.df.create(500): pass @@ -414,7 +408,6 @@ class TestRadosDiskFile(unittest.TestCase): (fcont[8:], 8)] assert(writes == check_list) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() def test_df_writer_put(self): meta = {'Content-Length': 0, @@ -430,7 +423,6 @@ class TestRadosDiskFile(unittest.TestCase): assert(ca == check_1) assert(meta['name'] == self._obj_name()) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() def test_df_write_metadata(self): meta = {'Content-Length': 0, @@ -444,7 +436,6 @@ class TestRadosDiskFile(unittest.TestCase): assert(ca == check_1) assert(meta['name'] == self._obj_name()) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() def test_df_delete(self): meta = {'name': self._obj_name(), 'Content-Length': 0, @@ -460,7 +451,6 @@ class TestRadosDiskFile(unittest.TestCase): finally: assert(success) self._assert_if_rados_not_opened() - self._assert_if_rados_not_closed() self.ioctx.remove_object.assert_called_once_with(self._obj_name())