From ea1cad2bedabc72604d935daeb359a1e6180d8f0 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Thu, 13 Sep 2018 14:17:54 +0300 Subject: [PATCH] Filesystem driver: add chunk size config option At the moment, the filesystem driver uses a hardcoded 64 KB chunk size, used when reading/writing image files. This can be extremely inefficient, especially when file shares are used. For this reason, this change will make it configurable. DocImpact Change-Id: I1559c03308d36ecf9305c7a72ad658ecd1f2dc76 Closes-Bug: #1792869 --- glance_store/_drivers/filesystem.py | 42 ++++++++++++------- .../tests/unit/test_filesystem_store.py | 24 +++++++---- .../tests/unit/test_multistore_filesystem.py | 8 +--- glance_store/tests/unit/test_opts.py | 1 + .../fs-drv-chunk-sz-a1b2f6a72fad92d5.yaml | 5 +++ 5 files changed, 51 insertions(+), 29 deletions(-) create mode 100644 releasenotes/notes/fs-drv-chunk-sz-a1b2f6a72fad92d5.yaml diff --git a/glance_store/_drivers/filesystem.py b/glance_store/_drivers/filesystem.py index 8e8481c4..420002a2 100644 --- a/glance_store/_drivers/filesystem.py +++ b/glance_store/_drivers/filesystem.py @@ -148,6 +148,23 @@ Possible values: Related options: * None +"""), + cfg.IntOpt('filesystem_store_chunk_size', + default=64 * units.Ki, + min=1, + help=""" +Chunk size, in bytes. + +The chunk size used when reading or writing image files. Raising this value +may improve the throughput but it may also slightly increase the memory usage +when handling a large number of requests. + +Possible Values: + * Any positive integer value + +Related options: + * None + """)] MULTI_FILESYSTEM_METADATA_SCHEMA = { @@ -243,8 +260,6 @@ class Store(glance_store.driver.Store): capabilities.BitMasks.WRITE_ACCESS | capabilities.BitMasks.DRIVER_REUSABLE) OPTIONS = _FILESYSTEM_CONFIGS - READ_CHUNKSIZE = 64 * units.Ki - WRITE_CHUNKSIZE = READ_CHUNKSIZE FILESYSTEM_STORE_METADATA = None def get_schemes(self): @@ -384,19 +399,18 @@ class Store(glance_store.driver.Store): itself, it should raise `exceptions.BadStoreConfiguration` """ if self.backend_group: - fdir = getattr( - self.conf, self.backend_group).filesystem_store_datadir - fdirs = getattr( - self.conf, self.backend_group).filesystem_store_datadirs - fstore_perm = getattr( - self.conf, self.backend_group).filesystem_store_file_perm - meta_file = getattr( - self.conf, self.backend_group).filesystem_store_metadata_file + store_conf = getattr(self.conf, self.backend_group) else: - fdir = self.conf.glance_store.filesystem_store_datadir - fdirs = self.conf.glance_store.filesystem_store_datadirs - fstore_perm = self.conf.glance_store.filesystem_store_file_perm - meta_file = self.conf.glance_store.filesystem_store_metadata_file + store_conf = self.conf.glance_store + + fdir = store_conf.filesystem_store_datadir + fdirs = store_conf.filesystem_store_datadirs + fstore_perm = store_conf.filesystem_store_file_perm + meta_file = store_conf.filesystem_store_metadata_file + + self.chunk_size = store_conf.filesystem_store_chunk_size + self.READ_CHUNKSIZE = self.chunk_size + self.WRITE_CHUNKSIZE = self.READ_CHUNKSIZE if not (fdir or fdirs): reason = (_("Specify at least 'filesystem_store_datadir' or " diff --git a/glance_store/tests/unit/test_filesystem_store.py b/glance_store/tests/unit/test_filesystem_store.py index 53e94e5c..ebc664c8 100644 --- a/glance_store/tests/unit/test_filesystem_store.py +++ b/glance_store/tests/unit/test_filesystem_store.py @@ -43,21 +43,15 @@ class TestStore(base.StoreBaseTest, def setUp(self): """Establish a clean test environment.""" super(TestStore, self).setUp() - self.orig_chunksize = filesystem.Store.READ_CHUNKSIZE - filesystem.Store.READ_CHUNKSIZE = 10 self.store = filesystem.Store(self.conf) self.config(filesystem_store_datadir=self.test_dir, + filesystem_store_chunk_size=10, stores=['glance.store.filesystem.Store'], group="glance_store") self.store.configure() self.register_store_schemes(self.store, 'file') self.hash_algo = 'sha256' - def tearDown(self): - """Clear the test environment.""" - super(TestStore, self).tearDown() - filesystem.ChunkedFile.CHUNKSIZE = self.orig_chunksize - def _create_metadata_json_file(self, metadata): expected_image_id = str(uuid.uuid4()) jsonfilename = os.path.join(self.test_dir, @@ -185,7 +179,10 @@ class TestStore(base.StoreBaseTest, def test_add_with_verifier(self): """Test that 'verifier.update' is called when verifier is provided.""" verifier = mock.MagicMock(name='mock_verifier') - self.store.chunk_size = units.Ki + self.config(filesystem_store_chunk_size=units.Ki, + group='glance_store') + self.store.configure() + image_id = str(uuid.uuid4()) file_size = units.Ki # 1K file_contents = b"*" * file_size @@ -741,3 +738,14 @@ class TestStore(base.StoreBaseTest, mode = os.stat(expected_location[len('file:/'):])[stat.ST_MODE] perm = int(str(self.conf.glance_store.filesystem_store_file_perm), 8) self.assertEqual(perm, stat.S_IMODE(mode)) + + def test_configure_add_chunk_size(self): + # This definitely won't be the default + chunk_size = units.Gi + self.config(filesystem_store_chunk_size=chunk_size, + group="glance_store") + self.store.configure_add() + + self.assertEqual(chunk_size, self.store.chunk_size) + self.assertEqual(chunk_size, self.store.READ_CHUNKSIZE) + self.assertEqual(chunk_size, self.store.WRITE_CHUNKSIZE) diff --git a/glance_store/tests/unit/test_multistore_filesystem.py b/glance_store/tests/unit/test_multistore_filesystem.py index a9ca3313..62d68e72 100644 --- a/glance_store/tests/unit/test_multistore_filesystem.py +++ b/glance_store/tests/unit/test_multistore_filesystem.py @@ -70,19 +70,13 @@ class TestMultiStore(base.MultiStoreBaseTest, self.test_dir = self.useFixture(fixtures.TempDir()).path self.addCleanup(self.conf.reset) - self.orig_chunksize = filesystem.Store.READ_CHUNKSIZE - filesystem.Store.READ_CHUNKSIZE = 10 self.store = filesystem.Store(self.conf, backend='file1') self.config(filesystem_store_datadir=self.test_dir, + filesystem_store_chunk_size=10, group="file1") self.store.configure() self.register_store_backend_schemes(self.store, 'file', 'file1') - def tearDown(self): - """Clear the test environment.""" - super(TestMultiStore, self).tearDown() - filesystem.ChunkedFile.CHUNKSIZE = self.orig_chunksize - def _create_metadata_json_file(self, metadata): expected_image_id = str(uuid.uuid4()) jsonfilename = os.path.join(self.test_dir, diff --git a/glance_store/tests/unit/test_opts.py b/glance_store/tests/unit/test_opts.py index c0e0f8af..e2598d23 100644 --- a/glance_store/tests/unit/test_opts.py +++ b/glance_store/tests/unit/test_opts.py @@ -85,6 +85,7 @@ class OptsTestCase(base.StoreBaseTest): 'cinder_volume_type', 'default_swift_reference', 'https_insecure', + 'filesystem_store_chunk_size', 'filesystem_store_datadir', 'filesystem_store_datadirs', 'filesystem_store_file_perm', diff --git a/releasenotes/notes/fs-drv-chunk-sz-a1b2f6a72fad92d5.yaml b/releasenotes/notes/fs-drv-chunk-sz-a1b2f6a72fad92d5.yaml new file mode 100644 index 00000000..887e563b --- /dev/null +++ b/releasenotes/notes/fs-drv-chunk-sz-a1b2f6a72fad92d5.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + The filesystem driver is now using a configurable chunk size. Increasing it + may avoid bottlenecks.