From 370562e1f1641fb403d8a537572cf4f3ce573b61 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). Resolved Conflicts: tox.ini unit_tests/test_rabbitmq_server_relations.py 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 | 7 ++++- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/hooks/rabbitmq_context.py b/hooks/rabbitmq_context.py index 95da30f6..8b67bcad 100644 --- a/hooks/rabbitmq_context.py +++ b/hooks/rabbitmq_context.py @@ -55,8 +55,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): @@ -203,9 +206,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 """ @@ -231,6 +234,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 0072bb72..cdae6beb 100644 --- a/unit_tests/test_rabbitmq_context.py +++ b/unit_tests/test_rabbitmq_context.py @@ -251,7 +251,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): @@ -276,11 +276,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 3981affd..1d291349 100644 --- a/unit_tests/test_rabbitmq_server_relations.py +++ b/unit_tests/test_rabbitmq_server_relations.py @@ -326,6 +326,8 @@ 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') @patch('rabbit_utils.nrpe.NRPE.add_check') @@ -355,7 +357,9 @@ class RelationUtil(CharmTestCase): mock_check_call, mock_remove_check, mock_add_check, mock_local_unit, - mock_create_user): + mock_create_user, + mock_grant_permissions, + mock_lsb_release): self.test_config.set('ssl', 'on') @@ -375,6 +379,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(