Repository: Duke-GCB/DukeDSClient

Issue:
Limit default upload/download workers to 8.
Right now we use the number of CPUs as the default value.
For cluster environments this is overkill and can result in lesser performance.
Changing it to limit default to 8 for upload_workers and download_workers.
We will still ask the number of CPUS just limit it.

Discussion:

Patch:
diff --git a/ddsc/config.py b/ddsc/config.py
index 22d09ec..7ccea6c 100644
--- a/ddsc/config.py
+++ b/ddsc/config.py
@@ -19,6 +19,7 @@ DDS_DEFAULT_UPLOAD_CHUNKS = 100 * MB_TO_BYTES
 AUTH_ENV_KEY_NAME = 'DUKE_DATA_SERVICE_AUTH'
 # when uploading skip .DS_Store, our key file, and ._ (resource fork metadata)
 FILE_EXCLUDE_REGEX_DEFAULT = '^\.DS_Store$|^\.ddsclient$|^\.\_'
+MAX_DEFAULT_WORKERS = 8
 
 
 def create_config():
@@ -32,6 +33,14 @@ def create_config():
     return config
 
 
+def default_num_workers():
+    """
+    Return the number of workers to use as default if not specified by a config file.
+    Returns the number of CPUs or MAX_DEFAULT_WORKERS (whichever is less).
+    """
+    return min(multiprocessing.cpu_count(), MAX_DEFAULT_WORKERS)
+
+
 class Config(object):
     """
     Global configuration object based on config files an environment variables.
@@ -124,7 +133,7 @@ class Config(object):
         Return the number of parallel works to use when uploading a file.
         :return: int number of workers. Specify None or 1 to disable parallel uploading
         """
-        return self.values.get(Config.UPLOAD_WORKERS, multiprocessing.cpu_count())
+        return self.values.get(Config.UPLOAD_WORKERS, default_num_workers())
 
     @property
     def download_workers(self):
@@ -133,7 +142,7 @@ class Config(object):
         :return: int number of workers. Specify None or 1 to disable parallel downloading
         """
         # Profiling download on different servers showed half the number of CPUs to be optimum for speed.
-        default_workers = int(math.ceil(multiprocessing.cpu_count()/2))
+        default_workers = int(math.ceil(default_num_workers()/2))
         return self.values.get(Config.DOWNLOAD_WORKERS, default_workers)
 
     @property


Test patch:
diff --git a/ddsc/tests/test_config.py b/ddsc/tests/test_config.py
index 21c87c8..f8ea55c 100644
--- a/ddsc/tests/test_config.py
+++ b/ddsc/tests/test_config.py
@@ -12,7 +12,7 @@ class TestConfig(TestCase):
         self.assertEqual(config.agent_key, None)
         self.assertEqual(config.auth, None)
         self.assertEqual(config.upload_bytes_per_chunk, ddsc.config.DDS_DEFAULT_UPLOAD_CHUNKS)
-        self.assertEqual(config.upload_workers, multiprocessing.cpu_count())
+        self.assertEqual(config.upload_workers, min(multiprocessing.cpu_count(), ddsc.config.MAX_DEFAULT_WORKERS))
 
     def test_global_then_local(self):
         config = ddsc.config.Config()
@@ -37,8 +37,9 @@ class TestConfig(TestCase):
         self.assertEqual(config.agent_key, '123')
         self.assertEqual(config.auth, 'secret')
         self.assertEqual(config.upload_bytes_per_chunk, 1293892)
-        self.assertEqual(config.upload_workers, multiprocessing.cpu_count())
-        self.assertEqual(config.download_workers, int(math.ceil(multiprocessing.cpu_count()/2)))
+        num_upload_workers = min(multiprocessing.cpu_count(), ddsc.config.MAX_DEFAULT_WORKERS)
+        self.assertEqual(config.upload_workers, num_upload_workers)
+        self.assertEqual(config.download_workers, int(math.ceil(num_upload_workers/2)))
 
         config.update_properties(local_config)
         self.assertEqual(config.url, 'dataservice2.com')
@@ -96,3 +97,11 @@ class TestConfig(TestCase):
         }
         for value, exp in value_and_expected:
             self.assertEqual(exp, ddsc.config.Config.parse_bytes_str(value))
+
+    def test_default_num_workers(self):
+        orig_max_default_workers = ddsc.config.MAX_DEFAULT_WORKERS
+        ddsc.config.MAX_DEFAULT_WORKERS = 5000
+        self.assertEqual(multiprocessing.cpu_count(), ddsc.config.default_num_workers())
+        ddsc.config.MAX_DEFAULT_WORKERS = 1
+        self.assertEqual(1, ddsc.config.default_num_workers())
+        ddsc.config.MAX_DEFAULT_WORKERS = orig_max_default_workers