Merge "Filesystem driver: add chunk size config option"

5 files changed, 51 insertions, 29 deletions

diff --git a/glance_store/_drivers/filesystem.py b/glance_store/_drivers/filesystem.py
index f9c3e47..4181f9d 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 53e94e5..ebc664c 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 a9ca331..62d68e7 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 c0e0f8a..e2598d2 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 0000000..887e563
--- /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.