[mthaddon,r=james-page] Add configuration for worker and per disk thread pool sizes

Make the number of workers for account, container and object servers a multiplier of the number of CPUs, and default to a sensible setting per recommendations in http://docs.openstack.org/developer/swift/deployment_guide.html#general-service-tuning. Also, add in a tunable option for object server threads_per_disk and tune default to recommendations from http://docs.openstack.org/developer/swift/deployment_guide.html#object-server-configuration.

config.yaml

@@ -49,4 +49,17 @@ options:
     default: 6002
     type: int
     description: Listening port of the swift-account-server.
-
+  worker-multiplier:
+    default: 1
+    type: int
+    description: |
+      The CPU multiplier to use when configuring worker processes for the
+      account, container and object server processes.
+  object-server-threads-per-disk:
+    default: 4
+    type: int
+    description: |
+      Size of the per-disk thread pool used for performing disk I/O. 0 means
+      to not use a per-disk thread pool. It is recommended to keep this value
+      small, as large values can result in high read latencies due to large
+      queue depths. A good starting point is 4 threads per disk.

hooks/swift_storage_context.py

@@ -61,10 +61,15 @@ class SwiftStorageServerContext(OSContextGenerator):
     interfaces = []
 
     def __call__(self):
+        import psutil
+        multiplier = int(config('worker-multiplier')) or 1
         ctxt = {
             'local_ip': unit_private_ip(),
             'account_server_port': config('account-server-port'),
             'container_server_port': config('container-server-port'),
             'object_server_port': config('object-server-port'),
+            'workers': str(psutil.NUM_CPUS * multiplier),
+            'object_server_threads_per_disk': config(
+                'object-server-threads-per-disk'),
         }
         return ctxt

hooks/swift_storage_utils.py

@@ -48,7 +48,7 @@ from charmhelpers.contrib.openstack import (
 
 PACKAGES = [
     'swift', 'swift-account', 'swift-container', 'swift-object',
-    'xfsprogs', 'gdisk', 'lvm2', 'python-jinja2',
+    'xfsprogs', 'gdisk', 'lvm2', 'python-jinja2', 'python-psutil',
 ]
 
 TEMPLATES = 'templates/'
@@ -166,7 +166,8 @@ def determine_block_devices():
 
     # attempt to ensure block devices, but filter out missing devs
     _none = ['None', 'none', None]
-    valid_bdevs = [x for x in map(ensure_block_device, bdevs) if x not in _none]
+    valid_bdevs = \
+        [x for x in map(ensure_block_device, bdevs) if x not in _none]
     log('Valid ensured block devices: %s' % valid_bdevs)
     return valid_bdevs
 

templates/account-server.conf

@@ -1,7 +1,7 @@
 [DEFAULT]
 bind_ip = 0.0.0.0
 bind_port = {{ account_server_port }}
-workers = 2
+workers = {{ workers }}
 
 [pipeline:main]
 pipeline = recon account-server

templates/container-server.conf

@@ -1,7 +1,7 @@
 [DEFAULT]
 bind_ip = 0.0.0.0
 bind_port = {{ container_server_port }}
-workers = 2
+workers = {{ workers }}
 
 [pipeline:main]
 pipeline = recon container-server

templates/object-server.conf

@@ -1,7 +1,7 @@
 [DEFAULT]
 bind_ip = 0.0.0.0
 bind_port = {{ object_server_port }}
-workers = 2
+workers = {{ workers }}
 
 [pipeline:main]
 pipeline = recon object-server
@@ -12,6 +12,7 @@ recon_cache_path = /var/cache/swift
 
 [app:object-server]
 use = egg:swift#object
+threads_per_disk = {{ object_server_threads_per_disk }}
 
 [object-replicator]
 

unit_tests/test_swift_storage_context.py

@@ -15,6 +15,7 @@ TO_PATCH = [
 
 
 class SwiftStorageContextTests(CharmTestCase):
+
     def setUp(self):
         super(SwiftStorageContextTests, self).setUp(swift_context, TO_PATCH)
         self.config.side_effect = self.test_config.get
@@ -56,16 +57,22 @@ class SwiftStorageContextTests(CharmTestCase):
             _file.write.assert_called_with('RSYNC_ENABLE=true\n')
 
     def test_swift_storage_server_context(self):
+        import psutil
         self.unit_private_ip.return_value = '10.0.0.5'
         self.test_config.set('account-server-port', '500')
         self.test_config.set('object-server-port', '501')
         self.test_config.set('container-server-port', '502')
+        self.test_config.set('object-server-threads-per-disk', '3')
+        self.test_config.set('worker-multiplier', '3')
+        num_workers = psutil.NUM_CPUS * 3
         ctxt = swift_context.SwiftStorageServerContext()
         result = ctxt()
         ex = {
             'container_server_port': '502',
             'object_server_port': '501',
             'account_server_port': '500',
-            'local_ip': '10.0.0.5'
+            'local_ip': '10.0.0.5',
+            'object_server_threads_per_disk': '3',
+            'workers': str(num_workers),
         }
         self.assertEquals(ex, result)

unit_tests/test_swift_storage_relations.py

@@ -40,6 +40,7 @@ TO_PATCH = [
 
 
 class SwiftStorageRelationsTests(CharmTestCase):
+
     def setUp(self):
         super(SwiftStorageRelationsTests, self).setUp(hooks,
                                                       TO_PATCH)

unit_tests/test_swift_storage_utils.py

@@ -63,6 +63,7 @@ SCRIPT_RC_ENV = {
 
 
 class SwiftStorageUtilsTests(CharmTestCase):
+
     def setUp(self):
         super(SwiftStorageUtilsTests, self).setUp(swift_utils, TO_PATCH)
         self.config.side_effect = self.test_config.get
@@ -92,7 +93,7 @@ class SwiftStorageUtilsTests(CharmTestCase):
         wgets = []
         for s in ['account', 'object', 'container']:
             _c = call(['wget', '%s/%s.ring.gz' % (url, s),
-                      '-O', '/etc/swift/%s.ring.gz' % s])
+                       '-O', '/etc/swift/%s.ring.gz' % s])
             wgets.append(_c)
         self.assertEquals(wgets, self.check_call.call_args_list)
 

unit_tests/test_utils.py

@@ -45,6 +45,7 @@ def get_default_config():
 
 
 class CharmTestCase(unittest.TestCase):
+
     def setUp(self, obj, patches):
         super(CharmTestCase, self).setUp()
         self.patches = patches
@@ -65,6 +66,7 @@ class CharmTestCase(unittest.TestCase):
 
 
 class TestConfig(object):
+
     def __init__(self):
         self.config = get_default_config()
 
@@ -80,12 +82,13 @@ class TestConfig(object):
         return self.config
 
     def set(self, attr, value):
-            if attr not in self.config:
-                raise KeyError
-            self.config[attr] = value
+        if attr not in self.config:
+            raise KeyError
+        self.config[attr] = value
 
 
 class TestRelation(object):
+
     def __init__(self, relation_data={}):
         self.relation_data = relation_data