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 <thiago@redhat.com>

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)

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):

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())