From 0ca8a6b4fe706473b983566f7cb10ee2a894a903 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 13 Sep 2024 17:03:26 +0000 Subject: [PATCH 1/6] Update max_concurrent default calculation --- CHANGELOG.rst | 6 ++++++ brewtils/config.py | 4 ++++ brewtils/specification.py | 7 +++++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e0c7bd9b..94b7d98e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,12 @@ Brewtils Changelog ================== +3.27.3 +------ +TBD + +- Updated Plugin `max_concurrent` to support -1 to utilize the default formula that `concurrent.futures.ThreadPoolExecutor` supports `min(32, os.cpu_count() + 4)` + 3.27.2 ------ 9/12/24 diff --git a/brewtils/config.py b/brewtils/config.py index a8320ef7..4050f70d 100644 --- a/brewtils/config.py +++ b/brewtils/config.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import json import logging +import os from argparse import ArgumentParser from yapconf import YapconfSpec @@ -155,6 +156,9 @@ def load_config( if "bg_url_prefix" in config: config.bg_url_prefix = normalize_url_prefix(config.bg_url_prefix) + if config.max_concurrent < 0: + config.max_concurrent = min(32, os.cpu_count() + 4) + return config diff --git a/brewtils/specification.py b/brewtils/specification.py index 2ae991c2..e3829563 100644 --- a/brewtils/specification.py +++ b/brewtils/specification.py @@ -209,8 +209,11 @@ def _is_json_dict(s): }, "max_concurrent": { "type": "int", - "description": "Maximum number of requests to process concurrently", - "default": 5, + "description": ( + "Maximum number of requests to process concurrently," + " -1 will default max concurrent to min(32, os.cpu_count() + 4)" + ), + "default": -1, }, "worker_shutdown_timeout": { "type": "int", From a6dbbda7ee1bd62d7ae9e7cad06bcc9898cf7f08 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 13 Sep 2024 17:09:24 +0000 Subject: [PATCH 2/6] Add checks --- brewtils/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/brewtils/config.py b/brewtils/config.py index 4050f70d..bf89f187 100644 --- a/brewtils/config.py +++ b/brewtils/config.py @@ -156,7 +156,7 @@ def load_config( if "bg_url_prefix" in config: config.bg_url_prefix = normalize_url_prefix(config.bg_url_prefix) - if config.max_concurrent < 0: + if "max_concurrent" in config and config.max_concurrent < 0: config.max_concurrent = min(32, os.cpu_count() + 4) return config From 6130479c386406b610024b390b2ac4a062305db9 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:35:00 +0000 Subject: [PATCH 3/6] Updating Testing --- brewtils/config.py | 2 +- test/config_test.py | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/brewtils/config.py b/brewtils/config.py index bf89f187..f5a65726 100644 --- a/brewtils/config.py +++ b/brewtils/config.py @@ -156,7 +156,7 @@ def load_config( if "bg_url_prefix" in config: config.bg_url_prefix = normalize_url_prefix(config.bg_url_prefix) - if "max_concurrent" in config and config.max_concurrent < 0: + if "max_concurrent" not in config or config.max_concurrent < 0: config.max_concurrent = min(32, os.cpu_count() + 4) return config diff --git a/test/config_test.py b/test/config_test.py index 8e5051b8..a1bc1e57 100644 --- a/test/config_test.py +++ b/test/config_test.py @@ -38,6 +38,25 @@ def params(): } +class TestMaxConcurrent(object): + + def test_negative_max_concurrent(self, monkeypatch, params): + monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) + + cli_args = ["-max_concurrent", "-1"] + config = load_config(cli_args=cli_args) + + assert config["max_concurrent"] == 8 + + def test_positive_max_concurrent(self, monkeypatch, params): + monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) + + cli_args = ["-max_concurrent", "3"] + config = load_config(cli_args=cli_args) + + assert config["max_concurrent"] == 3 + + class TestGetConnectionInfo(object): def test_kwargs(self, params): assert params == get_connection_info(**params) From fd23886dbf4606ad01d5986ef2bfb6dc1864a2d0 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:36:24 +0000 Subject: [PATCH 4/6] Fixing testing --- test/config_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/config_test.py b/test/config_test.py index a1bc1e57..d7d212f0 100644 --- a/test/config_test.py +++ b/test/config_test.py @@ -43,7 +43,7 @@ class TestMaxConcurrent(object): def test_negative_max_concurrent(self, monkeypatch, params): monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) - cli_args = ["-max_concurrent", "-1"] + cli_args = ["--bg-host", "the_host", "-max_concurrent", "-1"] config = load_config(cli_args=cli_args) assert config["max_concurrent"] == 8 @@ -51,7 +51,7 @@ def test_negative_max_concurrent(self, monkeypatch, params): def test_positive_max_concurrent(self, monkeypatch, params): monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) - cli_args = ["-max_concurrent", "3"] + cli_args = ["--bg-host", "the_host", "-max_concurrent", "3"] config = load_config(cli_args=cli_args) assert config["max_concurrent"] == 3 From e1c1e25cb6cc2f734ff5a77d5fea73759cd4e922 Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:37:28 +0000 Subject: [PATCH 5/6] fixed cli arg --- test/config_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/config_test.py b/test/config_test.py index d7d212f0..ce1535ce 100644 --- a/test/config_test.py +++ b/test/config_test.py @@ -43,7 +43,7 @@ class TestMaxConcurrent(object): def test_negative_max_concurrent(self, monkeypatch, params): monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) - cli_args = ["--bg-host", "the_host", "-max_concurrent", "-1"] + cli_args = ["--bg-host", "the_host", "-max-concurrent", "-1"] config = load_config(cli_args=cli_args) assert config["max_concurrent"] == 8 @@ -51,7 +51,7 @@ def test_negative_max_concurrent(self, monkeypatch, params): def test_positive_max_concurrent(self, monkeypatch, params): monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) - cli_args = ["--bg-host", "the_host", "-max_concurrent", "3"] + cli_args = ["--bg-host", "the_host", "-max-concurrent", "3"] config = load_config(cli_args=cli_args) assert config["max_concurrent"] == 3 From 1771f57754e1c48bf4a9b7d2f722ecd12274902b Mon Sep 17 00:00:00 2001 From: TheBurchLog <5104941+TheBurchLog@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:41:13 +0000 Subject: [PATCH 6/6] double dash args --- test/config_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/config_test.py b/test/config_test.py index ce1535ce..750514a7 100644 --- a/test/config_test.py +++ b/test/config_test.py @@ -43,15 +43,15 @@ class TestMaxConcurrent(object): def test_negative_max_concurrent(self, monkeypatch, params): monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) - cli_args = ["--bg-host", "the_host", "-max-concurrent", "-1"] + cli_args = ["--bg-host", "the_host", "--max-concurrent", "-1"] config = load_config(cli_args=cli_args) - assert config["max_concurrent"] == 8 + assert config["max_concurrent"] == 6 def test_positive_max_concurrent(self, monkeypatch, params): monkeypatch.setattr(os, "cpu_count", Mock(return_value=2)) - cli_args = ["--bg-host", "the_host", "-max-concurrent", "3"] + cli_args = ["--bg-host", "the_host", "--max-concurrent", "3"] config = load_config(cli_args=cli_args) assert config["max_concurrent"] == 3