From 552eb3a5d212d6121a894bd36b5f58870e18b92d Mon Sep 17 00:00:00 2001 From: Vern Hart Date: Thu, 3 May 2018 21:46:22 +0000 Subject: [PATCH] Enforce a maximum of 1024 async threads. The beam.smp process won't start if more than 1024 are configured, the charm could make this by default on large systems (e.g. more than 42 CPUs). This change makes RabbitMQEnvContext.calculate_threads() never return more than 1024 (MAX_NUM_THREADS). Change-Id: I92879445210bac6ee7d96a704cdf428ca738e3b6 Closes-Bug: #1768986 (cherry picked from commit 3c1c05ee598b2fc85ddf0f971528dde433312bff) --- hooks/rabbitmq_context.py | 11 +++++-- unit_tests/test_rabbitmq_context.py | 30 ++++++++++++++++++-- unit_tests/test_rabbitmq_server_relations.py | 5 +++- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/hooks/rabbitmq_context.py b/hooks/rabbitmq_context.py index 1d0087e2..14280eb8 100644 --- a/hooks/rabbitmq_context.py +++ b/hooks/rabbitmq_context.py @@ -57,8 +57,11 @@ ENV_CONF = '/etc/rabbitmq/rabbitmq-env.conf' # https://tinyurl.com/rabbitmq-3-5-7 for exact value. Note that # this default has increased with newer versions so we should # track this and keep the charm up-to-date. +# Also note that 1024 is the maximum supported number of +# threads. DEFAULT_MULTIPLIER = 24 MAX_DEFAULT_THREADS = DEFAULT_MULTIPLIER * 2 +MAX_NUM_THREADS = 1024 def convert_from_base64(v): @@ -210,9 +213,9 @@ class RabbitMQEnvContext(object): Determine the number of erl vm threads in pool based in cpu resources available. - Number of threads will be limited to MAX_DEFAULT_WORKERS in + Number of threads will be limited to MAX_DEFAULT_THREADS in container environments where no worker-multipler configuration - option been set. + option been set and to MAX_NUM_THREADS in all other situations. @returns int: number of io threads to allocate """ @@ -238,6 +241,10 @@ class RabbitMQEnvContext(object): # configuration in LXD containers on large servers. count = min(count, MAX_DEFAULT_THREADS) + # Never configure a number of threads above the MAX_NUM_THREADS since + # otherwise the daemon won't start. + count = min(count, MAX_NUM_THREADS) + log("erl vm io thread pool size = {} (capped={})" .format(count, is_container()), DEBUG) diff --git a/unit_tests/test_rabbitmq_context.py b/unit_tests/test_rabbitmq_context.py index f165dd6a..309e871c 100644 --- a/unit_tests/test_rabbitmq_context.py +++ b/unit_tests/test_rabbitmq_context.py @@ -315,7 +315,7 @@ class TestRabbitMQEnvContext(unittest.TestCase): "'+A 48'"}) @mock.patch.object(rabbitmq_context, 'relation_ids', lambda *args: []) - @mock.patch.object(rabbitmq_context.psutil, 'NUM_CPUS', 128) + @mock.patch.object(rabbitmq_context.psutil, 'NUM_CPUS', 40) @mock.patch.object(rabbitmq_context, 'service_name') @mock.patch.object(rabbitmq_context, 'config') def test_rabbitmqenv_in_container(self, mock_config, mock_service_name): @@ -340,11 +340,35 @@ class TestRabbitMQEnvContext(unittest.TestCase): ctxt = rabbitmq_context.RabbitMQEnvContext()() self.assertEqual(ctxt['settings'], {'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS': - "'+A 3072'"}) + "'+A 960'"}) del config['erl-vm-io-thread-multiplier'] mock_is_ctnr.return_value = False ctxt = rabbitmq_context.RabbitMQEnvContext()() self.assertEqual(ctxt['settings'], {'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS': - "'+A 3072'"}) + "'+A 960'"}) + + @mock.patch.object(rabbitmq_context.psutil, 'NUM_CPUS', 48) + @mock.patch.object(rabbitmq_context, 'relation_ids', lambda *args: []) + @mock.patch.object(rabbitmq_context, 'service_name') + @mock.patch.object(rabbitmq_context, 'config') + def test_rabbitmqenv_max_num_threads(self, mock_config, mock_service_name): + config = {} + + def fake_config(key): + return config.get(key) + + mock_service_name.return_value = 'svc_foo' + mock_config.side_effect = fake_config + + ctxt = rabbitmq_context.RabbitMQEnvContext()() + self.assertEqual(ctxt['settings'], + {'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS': + "'+A 1024'"}) + + config['erl-vm-io-thread-multiplier'] = 10 + ctxt = rabbitmq_context.RabbitMQEnvContext()() + self.assertEqual(ctxt['settings'], + {'RABBITMQ_SERVER_ADDITIONAL_ERL_ARGS': + "'+A 480'"}) diff --git a/unit_tests/test_rabbitmq_server_relations.py b/unit_tests/test_rabbitmq_server_relations.py index 312dfbe9..d86ffbe0 100644 --- a/unit_tests/test_rabbitmq_server_relations.py +++ b/unit_tests/test_rabbitmq_server_relations.py @@ -328,6 +328,7 @@ class RelationUtil(CharmTestCase): if os.path.exists(tmpdir): shutil.rmtree(tmpdir) + @patch('rabbit_utils.lsb_release') @patch.object(rabbitmq_server_relations.rabbit, 'grant_permissions') @patch('rabbit_utils.create_user') @patch('rabbit_utils.local_unit') @@ -359,7 +360,8 @@ class RelationUtil(CharmTestCase): mock_remove_check, mock_add_check, mock_local_unit, mock_create_user, - mock_grant_permissions): + mock_grant_permissions, + mock_lsb_release): self.test_config.set('ssl', 'on') @@ -379,6 +381,7 @@ class RelationUtil(CharmTestCase): mock_nrpe_relation_ids.side_effect = lambda x: [ 'nrpe-external-master:1'] mock_local_unit.return_value = 'unit/0' + mock_lsb_release.return_value = {'DISTRIB_CODENAME': 'focal'} rabbitmq_server_relations.update_nrpe_checks() mock_check_output.assert_any_call(