From 27ab8a6aeb7a59bc56ed0ad26ffb5a83d426b447 Mon Sep 17 00:00:00 2001 From: Rajat Dhasmana Date: Thu, 13 Oct 2022 14:05:02 +0000 Subject: [PATCH] RBD: Wrap RBD calls in native threads librbd methods call lower level C code which runs in native thread and isn't aware about the eventlet threads hence hangs the eventlet loop until the native thread is executed. This could cause problems when we are creating multiple images with large size where one call to librados can cause the process to hang and other operations can starve for execution and error out. This patch wraps each RBD call in it's own native thread that won't affect other RBD call from executing. Change-Id: I8efb0460df9fcba050b5ce949eb10caea325c851 --- glance_store/_drivers/rbd.py | 12 ++++-- glance_store/tests/unit/test_rbd_store.py | 50 +++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/glance_store/_drivers/rbd.py b/glance_store/_drivers/rbd.py index f45a9098..2dd6ac51 100644 --- a/glance_store/_drivers/rbd.py +++ b/glance_store/_drivers/rbd.py @@ -22,6 +22,7 @@ import logging import math import urllib +from eventlet import tpool from oslo_config import cfg from oslo_utils import encodeutils from oslo_utils import units @@ -290,6 +291,9 @@ class Store(driver.Store): def get_schemes(self): return ('rbd',) + def RBDProxy(self): + return tpool.Proxy(rbd.RBD()) + @contextlib.contextmanager def get_connection(self, conffile, rados_id): client = rados.Rados(conffile=conffile, rados_id=rados_id) @@ -431,12 +435,12 @@ class Store(driver.Store): :returns: `glance_store.rbd.StoreLocation` object """ - librbd = rbd.RBD() features = conn.conf_get('rbd_default_features') if ((features is None) or (int(features) == 0)): features = rbd.RBD_FEATURE_LAYERING - librbd.create(ioctx, image_name, size, order, old_format=False, - features=int(features)) + self.RBDProxy().create(ioctx, image_name, size, order, + old_format=False, + features=int(features)) return StoreLocation({ 'fsid': fsid, 'pool': self.pool, @@ -496,7 +500,7 @@ class Store(driver.Store): raise exceptions.InUseByStore() # Then delete image. - rbd.RBD().remove(ioctx, image_name) + self.RBDProxy().remove(ioctx, image_name) except rbd.ImageHasSnapshots: log_msg = (_LW("Remove image %(img_name)s failed. " "It has snapshot(s) left.") % diff --git a/glance_store/tests/unit/test_rbd_store.py b/glance_store/tests/unit/test_rbd_store.py index 08ade51d..09eddc46 100644 --- a/glance_store/tests/unit/test_rbd_store.py +++ b/glance_store/tests/unit/test_rbd_store.py @@ -692,3 +692,53 @@ class TestStore(base.StoreBaseTest, self.assertEqual(self.called_commands_expected, self.called_commands_actual) super(TestStore, self).tearDown() + + def test_create_image_in_native_thread(self): + # Tests that we use non-0 features from ceph.conf and cast to int. + fsid = 'fake' + features = '3' + conf_get_mock = mock.Mock(return_value=features) + conn = mock.Mock(conf_get=conf_get_mock) + ioctxt = mock.sentinel.ioctxt + name = '1' + size = 1024 + order = 3 + fake_proxy = mock.MagicMock() + fake_rbd = mock.MagicMock() + + with mock.patch.object(rbd_store.tpool, 'Proxy') as tpool_mock, \ + mock.patch.object(rbd_store.rbd, 'RBD') as rbd_mock: + tpool_mock.return_value = fake_proxy + rbd_mock.return_value = fake_rbd + location = self.store._create_image( + fsid, conn, ioctxt, name, size, order) + self.assertEqual(fsid, location.specs['fsid']) + self.assertEqual(rbd_store.DEFAULT_POOL, location.specs['pool']) + self.assertEqual(name, location.specs['image']) + self.assertEqual(rbd_store.DEFAULT_SNAPNAME, + location.specs['snapshot']) + + tpool_mock.assert_called_once_with(fake_rbd) + fake_proxy.create.assert_called_once_with(ioctxt, name, size, order, + old_format=False, features=3) + + def test_delete_image_in_native_thread(self): + fake_proxy = mock.MagicMock() + fake_rbd = mock.MagicMock() + fake_ioctx = mock.MagicMock() + + with mock.patch.object(rbd_store.tpool, 'Proxy') as tpool_mock, \ + mock.patch.object(rbd_store.rbd, 'RBD') as rbd_mock, \ + mock.patch.object(self.store, 'get_connection') as mock_conn: + + mock_get_conn = mock_conn.return_value.__enter__.return_value + mock_ioctx = mock_get_conn.open_ioctx.return_value.__enter__ + mock_ioctx.return_value = fake_ioctx + tpool_mock.return_value = fake_proxy + rbd_mock.return_value = fake_rbd + + self.store._delete_image('fake_pool', self.location.image) + + tpool_mock.assert_called_once_with(fake_rbd) + fake_proxy.remove.assert_called_once_with(fake_ioctx, + self.location.image)