Setting special limits for Python 3.6 and 3.7 (#890)

* Setting special limits for Python 3.[6|7]
* Updated CODEOWNERS file

Author: vrdmr

CODEOWNERS

@@ -7,6 +7,7 @@
 
 #
 # AZURE FUNCTIONS TEAM
-# For all file changes, github would automatically include the following people in the PRs.
+# For all file changes, github would automatically
+# include the following people in the PRs.
 #
-* @anirudhgarg @Hazhzeng @vrdmr @AnatoliB
+* @vrdmr @AnatoliB

azure_functions_worker/constants.py

@@ -39,6 +39,8 @@
 PYTHON_THREADPOOL_THREAD_COUNT_DEFAULT = 1
 PYTHON_THREADPOOL_THREAD_COUNT_MIN = 1
 PYTHON_THREADPOOL_THREAD_COUNT_MAX = sys.maxsize
+PYTHON_THREADPOOL_THREAD_COUNT_MAX_37 = 32
+
 PYTHON_ISOLATE_WORKER_DEPENDENCIES_DEFAULT = False
 PYTHON_ISOLATE_WORKER_DEPENDENCIES_DEFAULT_39 = False
 PYTHON_ENABLE_WORKER_EXTENSIONS_DEFAULT = False

azure_functions_worker/dispatcher.py

@@ -26,7 +26,7 @@
 from . import protos
 from .constants import (PYTHON_THREADPOOL_THREAD_COUNT,
                         PYTHON_THREADPOOL_THREAD_COUNT_DEFAULT,
-                        PYTHON_THREADPOOL_THREAD_COUNT_MAX,
+                        PYTHON_THREADPOOL_THREAD_COUNT_MAX_37,
                         PYTHON_THREADPOOL_THREAD_COUNT_MIN)
 from .logging import disable_console_logging, enable_console_logging
 from .logging import (logger, error_logger, is_system_log_category,
@@ -567,25 +567,28 @@ def tp_max_workers_validator(value: str) -> bool:
                                'integer')
                 return False
 
-            if int_value < PYTHON_THREADPOOL_THREAD_COUNT_MIN or (
-                    int_value > PYTHON_THREADPOOL_THREAD_COUNT_MAX):
+            if int_value < PYTHON_THREADPOOL_THREAD_COUNT_MIN:
                 logger.warning(f'{PYTHON_THREADPOOL_THREAD_COUNT} must be set '
                                f'to a value between '
                                f'{PYTHON_THREADPOOL_THREAD_COUNT_MIN} and '
-                               f'{PYTHON_THREADPOOL_THREAD_COUNT_MAX}. '
-                               'Reverting to default value for max_workers')
+                               'sys.maxint. Reverting to default value for '
+                               'max_workers')
                 return False
-
             return True
 
         # Starting Python 3.9, worker won't be putting a limit on the
         # max_workers count in the created threadpool.
         default_value = None if sys.version_info.minor == 9 \
             else f'{PYTHON_THREADPOOL_THREAD_COUNT_DEFAULT}'
+
         max_workers = get_app_setting(setting=PYTHON_THREADPOOL_THREAD_COUNT,
                                       default_value=default_value,
                                       validator=tp_max_workers_validator)
 
+        if sys.version_info.minor <= 7:
+            max_workers = min(int(max_workers),
+                              PYTHON_THREADPOOL_THREAD_COUNT_MAX_37)
+
         # We can box the app setting as int for earlier python versions.
         return int(max_workers) if max_workers else None
 

tests/unittests/test_dispatcher.py

@@ -11,8 +11,7 @@
 from azure_functions_worker import testutils
 from azure_functions_worker.constants import PYTHON_THREADPOOL_THREAD_COUNT, \
     PYTHON_THREADPOOL_THREAD_COUNT_DEFAULT, \
-    PYTHON_THREADPOOL_THREAD_COUNT_MAX, \
-    PYTHON_THREADPOOL_THREAD_COUNT_MIN
+    PYTHON_THREADPOOL_THREAD_COUNT_MAX_37, PYTHON_THREADPOOL_THREAD_COUNT_MIN
 
 SysVersionInfo = col.namedtuple("VersionInfo", ["major", "minor", "micro",
                                                 "releaselevel", "serial"])
@@ -37,7 +36,8 @@ def setUp(self):
             script_root=DISPATCHER_FUNCTIONS_DIR)
         self._default_workers: Optional[
             int] = PYTHON_THREADPOOL_THREAD_COUNT_DEFAULT
-        self._allowed_max_workers: int = 100000
+        self._over_max_workers: int = 10000
+        self._allowed_max_workers: int = PYTHON_THREADPOOL_THREAD_COUNT_MAX_37
         self._pre_env = dict(os.environ)
         self.mock_version_info = patch(
             'azure_functions_worker.dispatcher.sys.version_info',
@@ -128,33 +128,26 @@ async def test_dispatcher_sync_threadpool_below_min_setting(self):
                 await self._assert_workers_threadpool(self._ctrl, host,
                                                       self._default_workers)
             mock_logger.warning.assert_any_call(
-                f'{PYTHON_THREADPOOL_THREAD_COUNT} must be set to a value '
-                f'between {PYTHON_THREADPOOL_THREAD_COUNT_MIN} and '
-                f'{PYTHON_THREADPOOL_THREAD_COUNT_MAX}. Reverting to default '
-                f'value for max_workers')
+                f'{PYTHON_THREADPOOL_THREAD_COUNT} must be set '
+                f'to a value between '
+                f'{PYTHON_THREADPOOL_THREAD_COUNT_MIN} and '
+                'sys.maxint. Reverting to default value for '
+                'max_workers')
 
-    @unittest.skip("We no more check any max limit. This is up to the customer,"
-                   " how ever high int they want to set")
     async def test_dispatcher_sync_threadpool_exceed_max_setting(self):
-        """Test if the sync threadpool will pick up default value when the
+        """Test if the sync threadpool will pick up default max value when the
         setting is above maximum
         """
-        with patch('azure_functions_worker.dispatcher.logger') as mock_logger:
+        with patch('azure_functions_worker.dispatcher.logger'):
             # Configure thread pool max worker to an invalid value
             os.environ.update({PYTHON_THREADPOOL_THREAD_COUNT:
                               f'{self._over_max_workers}'})
             async with self._ctrl as host:
                 await self._check_if_function_is_ok(host)
 
-                # Ensure the dispatcher sync threadpool should fallback to 1
+                # Ensure the dispatcher sync threadpool should fallback to max
                 await self._assert_workers_threadpool(self._ctrl, host,
-                                                      self._default_workers)
-
-            mock_logger.warning.assert_any_call(
-                f'{PYTHON_THREADPOOL_THREAD_COUNT} must be set to a value '
-                f'between {PYTHON_THREADPOOL_THREAD_COUNT_MIN} and '
-                f'{PYTHON_THREADPOOL_THREAD_COUNT_MAX}. Reverting to default '
-                f'value for max_workers')
+                                                      self._allowed_max_workers)
 
     async def test_dispatcher_sync_threadpool_in_placeholder(self):
         """Test if the sync threadpool will pick up app setting in placeholder
@@ -189,13 +182,13 @@ async def test_dispatcher_sync_threadpool_in_placeholder_invalid(self):
                 mock_logger.warning.assert_any_call(
                     f'{PYTHON_THREADPOOL_THREAD_COUNT} must be an integer')
 
-    @unittest.skip("We no more check any max limit. This is up to the customer,"
-                   " how ever high int they want to set")
     async def test_dispatcher_sync_threadpool_in_placeholder_above_max(self):
-        """Test if the sync threadpool will use the default setting when the
-        app setting is above maximum
+        """Test if the sync threadpool will use the default max setting when
+        the app setting is above maximum.
+
+        Note: This is designed for Linux Consumption.
         """
-        with patch('azure_functions_worker.dispatcher.logger') as mock_logger:
+        with patch('azure_functions_worker.dispatcher.logger'):
             async with self._ctrl as host:
                 await self._check_if_function_is_ok(host)
 
@@ -204,13 +197,7 @@ async def test_dispatcher_sync_threadpool_in_placeholder_above_max(self):
                     PYTHON_THREADPOOL_THREAD_COUNT: f'{self._over_max_workers}'
                 })
                 await self._assert_workers_threadpool(self._ctrl, host,
-                                                      self._default_workers)
-
-                mock_logger.warning.assert_any_call(
-                    f'{PYTHON_THREADPOOL_THREAD_COUNT} must be set to a '
-                    f'value '
-                    'between 1 and 1024. '
-                    'Reverting to default value for max_workers')
+                                                      self._allowed_max_workers)
 
     async def test_dispatcher_sync_threadpool_in_placeholder_below_min(self):
         """Test if the sync threadpool will use the default setting when the
@@ -229,10 +216,11 @@ async def test_dispatcher_sync_threadpool_in_placeholder_below_min(self):
                                                       self._default_workers)
 
                 mock_logger.warning.assert_any_call(
-                    f'{PYTHON_THREADPOOL_THREAD_COUNT} must be set to a value '
-                    f'between {PYTHON_THREADPOOL_THREAD_COUNT_MIN} and '
-                    f'{PYTHON_THREADPOOL_THREAD_COUNT_MAX}. Reverting to '
-                    f'default value for max_workers')
+                    f'{PYTHON_THREADPOOL_THREAD_COUNT} must be set '
+                    f'to a value between '
+                    f'{PYTHON_THREADPOOL_THREAD_COUNT_MIN} and '
+                    'sys.maxint. Reverting to default value for '
+                    'max_workers')
 
     async def test_sync_invocation_request_log(self):
         with patch('azure_functions_worker.dispatcher.logger') as mock_logger:
@@ -418,32 +406,52 @@ def setUp(self):
         self.mock_version_info = patch(
             'azure_functions_worker.dispatcher.sys.version_info',
             SysVersionInfo(3, 8, 0, 'final', 0))
+        self._over_max_workers: int = 10000
+        self._allowed_max_workers: int = self._over_max_workers
         self.mock_version_info.start()
 
     def tearDown(self):
         os.environ.clear()
         os.environ.update(self._pre_env)
         self.mock_version_info.stop()
 
+    async def test_dispatcher_sync_threadpool_in_placeholder_above_max(self):
+        """Test if the sync threadpool will use any value and there isn't any
+        artificial max value set.
+        """
+        with patch('azure_functions_worker.dispatcher.logger'):
+            async with self._ctrl as host:
+                await self._check_if_function_is_ok(host)
+
+                # Reload environment variable on specialization
+                await host.reload_environment(environment={
+                    PYTHON_THREADPOOL_THREAD_COUNT: f'{self._over_max_workers}'
+                })
+                await self._assert_workers_threadpool(self._ctrl, host,
+                                                      self._allowed_max_workers)
+                self.assertNotEqual(
+                    self._ctrl._worker.get_sync_tp_workers_set(),
+                    self._default_workers)
+
 
 @unittest.skipIf(sys.version_info.minor != 9,
                  "Run the tests only for Python 3.9. In other platforms, "
                  "as the default passed is None, the cpu_count determines the "
                  "number of max_workers and we cannot mock the os.cpu_count() "
                  "in the concurrent.futures.ThreadPoolExecutor")
-class TestThreadPoolSettingsPython39(TestThreadPoolSettingsPython37):
+class TestThreadPoolSettingsPython39(TestThreadPoolSettingsPython38):
     def setUp(self):
         super(TestThreadPoolSettingsPython39, self).setUp()
 
         self.mock_os_cpu = patch(
             'os.cpu_count', return_value=2)
-        self.mock_os_cpu.start()
         # 6 - based on 2 cores - min(32, (os.cpu_count() or 1) + 4) - 2 + 4
         self._default_workers: Optional[int] = 6
-
         self.mock_version_info = patch(
             'azure_functions_worker.dispatcher.sys.version_info',
             SysVersionInfo(3, 9, 0, 'final', 0))
+
+        self.mock_os_cpu.start()
         self.mock_version_info.start()
 
     def tearDown(self):