From 5952b3de01786d9b2ea90b1a48c265fbbe067928 Mon Sep 17 00:00:00 2001 From: Trent Lloyd Date: Mon, 30 Nov 2020 15:40:54 +0800 Subject: [PATCH] Raise file limits to allow max-connections >4190 Install a systemd unit override with LimitNOFILE raised to the required number based on the same calculation the server performs to ensure both the requested max_connections and table_open_cache values can be met. The MySQL server does make some attempt to do this itself when started as root which worked under Xenial, however in Bionic the systemd service is started as the mysql user with LimitNOFILE=5000. As a result, the server cannot raise the limit itself and caps max-connections to 4190, table-open-cache to 200 and logs a warning to that effect: [Warning] Changed limits: max_connections: 4190 (requested 12000) More background: https://www.percona.com/blog/2017/10/12/open_files_limit-mystery/ Change-Id: I25b429cc9b4970e3d7ef39bb9e6d738fe943686f Closes-Bug: #1905366 --- hooks/percona_hooks.py | 42 ++++++++++-- templates/charm-nofile.conf | 2 + unit_tests/test_percona_hooks.py | 106 ++++++++++++++++++++++++++++--- 3 files changed, 137 insertions(+), 13 deletions(-) create mode 100644 templates/charm-nofile.conf diff --git a/hooks/percona_hooks.py b/hooks/percona_hooks.py index 99e0b4d..e41a290 100755 --- a/hooks/percona_hooks.py +++ b/hooks/percona_hooks.py @@ -50,6 +50,7 @@ from charmhelpers.core.host import ( mkdir, CompareHostReleases, pwgen, + init_is_systemd ) from charmhelpers.core.templating import render from charmhelpers.fetch import ( @@ -166,6 +167,8 @@ RES_MONITOR_PARAMS = ('params user="sstuser" password="%(sstpass)s" ' 'OCF_CHECK_LEVEL="1" ' 'meta migration-threshold=INFINITY failure-timeout=5s') +SYSTEMD_OVERRIDE_PATH = '/etc/systemd/system/mysql.service.d/charm-nofile.conf' + MYSQL_SOCKET = "/var/run/mysqld/mysqld.sock" @@ -211,6 +214,24 @@ def install(): install_mysql_ocf() +def render_override(ctx): + # max_connections/table_open_cache are shrunk to fit within ulimits. + # The following formula is taken from sql/mysqld.cc. + if init_is_systemd(): + open_files_limit = max( + (ctx['max_connections'] + 1) + 10 + (ctx['table_open_cache']*2), + (ctx['max_connections'] + 1) * 5, + 5000) + if not os.path.exists(os.path.dirname(SYSTEMD_OVERRIDE_PATH)): + os.makedirs(os.path.dirname(SYSTEMD_OVERRIDE_PATH)) + pre_hash = file_hash(SYSTEMD_OVERRIDE_PATH) + render(os.path.basename(SYSTEMD_OVERRIDE_PATH), + SYSTEMD_OVERRIDE_PATH, + {'open_files_limit': open_files_limit}) + if pre_hash != file_hash(SYSTEMD_OVERRIDE_PATH): + subprocess.check_call(['systemctl', 'daemon-reload']) + + def render_config(hosts=None): if hosts is None: hosts = [] @@ -277,8 +298,10 @@ def render_config(hosts=None): context.update(PerconaClusterHelper().parse_config()) render(os.path.basename(config_file), config_file, context, perms=0o444) + render_override(context) -def render_config_restart_on_changed(hosts, bootstrap=False): + +def render_config_restart_on_changed(hosts): """Render mysql config and restart mysql service if file changes as a result. @@ -290,12 +313,22 @@ def render_config_restart_on_changed(hosts, bootstrap=False): it is started so long as the new node to be added is guaranteed to have been restarted so as to apply the new config. """ + if is_leader() and not is_leader_bootstrapped(): + bootstrap = True + else: + bootstrap = False + config_file = resolve_cnf_file() - pre_hash = file_hash(config_file) + pre_hash_config = file_hash(config_file) + pre_hash_override = file_hash(SYSTEMD_OVERRIDE_PATH) render_config(hosts) create_binlogs_directory() update_db_rels = False - if file_hash(config_file) != pre_hash or bootstrap: + + hashes_changed = (file_hash(config_file) != pre_hash_config) or \ + (file_hash(SYSTEMD_OVERRIDE_PATH) != pre_hash_override) + + if hashes_changed or bootstrap: if bootstrap: bootstrap_pxc() # NOTE(dosaboy): this will not actually do anything if no cluster @@ -584,8 +617,7 @@ def config_changed(): log("Leader unit - bootstrap required={}" .format(not leader_bootstrapped), DEBUG) - render_config_restart_on_changed(hosts, - bootstrap=not leader_bootstrapped) + render_config_restart_on_changed(hosts) elif (leader_bootstrapped and is_sufficient_peers() and not cluster_series_upgrading): diff --git a/templates/charm-nofile.conf b/templates/charm-nofile.conf new file mode 100644 index 0000000..4c60f14 --- /dev/null +++ b/templates/charm-nofile.conf @@ -0,0 +1,2 @@ +[Service] +LimitNOFILE={{ open_files_limit }} diff --git a/unit_tests/test_percona_hooks.py b/unit_tests/test_percona_hooks.py index ca26419..1162ecc 100644 --- a/unit_tests/test_percona_hooks.py +++ b/unit_tests/test_percona_hooks.py @@ -471,16 +471,14 @@ class TestConfigChanged(CharmTestCase): self.get_cluster_hosts.return_value = [] hooks.config_changed() self.install_percona_xtradb_cluster.assert_called_once() - self.render_config_restart_on_changed.assert_called_once_with( - [], bootstrap=True) + self.render_config_restart_on_changed.assert_called_once_with([]) # Render without peers, leader bootstrapped self.is_leader_bootstrapped.return_value = True self.get_cluster_hosts.return_value = [] self.render_config_restart_on_changed.reset_mock() hooks.config_changed() - self.render_config_restart_on_changed.assert_called_once_with( - [], bootstrap=False) + self.render_config_restart_on_changed.assert_called_once_with([]) # Render without hosts, leader bootstrapped, never clustered self.is_leader_bootstrapped.return_value = True @@ -488,8 +486,7 @@ class TestConfigChanged(CharmTestCase): self.render_config_restart_on_changed.reset_mock() hooks.config_changed() - self.render_config_restart_on_changed.assert_called_once_with( - [], bootstrap=False) + self.render_config_restart_on_changed.assert_called_once_with([]) # Clustered at least once self.clustered_once.return_value = True @@ -501,7 +498,7 @@ class TestConfigChanged(CharmTestCase): self.render_config_restart_on_changed.reset_mock() hooks.config_changed() self.render_config_restart_on_changed.assert_called_once_with( - ['10.10.10.20', '10.10.10.30'], bootstrap=False) + ['10.10.10.20', '10.10.10.30']) # In none of the prior scenarios should update_root_password have been # called. @@ -515,7 +512,7 @@ class TestConfigChanged(CharmTestCase): self.render_config_restart_on_changed.reset_mock() hooks.config_changed() self.render_config_restart_on_changed.assert_called_once_with( - ['10.10.10.20', '10.10.10.30'], bootstrap=False) + ['10.10.10.20', '10.10.10.30']) self.update_root_password.assert_called_once() def test_config_changed_render_non_leader(self): @@ -810,6 +807,7 @@ class TestConfigs(CharmTestCase): TO_PATCH = [ 'config', 'is_leader', + 'render_override', ] def setUp(self): @@ -1019,6 +1017,98 @@ class TestConfigs(CharmTestCase): perms=0o444) +class TestRenderConfigRestartOnChanged(CharmTestCase): + TO_PATCH = [ + 'is_leader', + 'is_leader_bootstrapped', + 'bootstrap_pxc', + 'resolve_cnf_file', + 'file_hash', + 'render_config', + 'create_binlogs_directory', + 'bootstrap_pxc', + 'notify_bootstrapped', + 'service_running', + 'service_stop', + 'service_restart', + 'cluster_wait', + 'mark_seeded', + 'cluster_wait', + 'update_client_db_relations', + ] + + def setUp(self): + CharmTestCase.setUp(self, hooks, self.TO_PATCH) + self.is_leader.return_value = False + self.is_leader_bootstrapped.return_value = False + self.bootstrap_pxc.return_value = None + self.service_running.return_value = True + self.resolve_cnf_file.return_value = \ + '/etc/mysql/percona-xtradb-cluster.conf.d/mysqld.cnf' + self.file_hash.return_value = 'original' + self.service_restart.return_value = True + + def _render_config_changed(self, hosts): + self.file_hash.return_value = 'changed' + + def test_bootstrap_leader(self): + self.is_leader.return_value = True + self.is_leader_bootstrapped.return_value = False + self.render_config.side_effect = self._render_config_changed + + hooks.render_config_restart_on_changed([]) + + self.bootstrap_pxc.assert_called_once() + self.service_restart.assert_not_called() + + def test_bootstrapped_leader(self): + self.is_leader.return_value = True + self.is_leader_bootstrapped.return_value = True + self.render_config.side_effect = self._render_config_changed + + hooks.render_config_restart_on_changed([]) + + self.bootstrap_pxc.assert_not_called() + self.service_restart.assert_called_once() + + def test_noleader(self): + self.is_leader.return_value = False + self.is_leader_bootstrapped.return_value = False + self.render_config.side_effect = self._render_config_changed + + hooks.render_config_restart_on_changed([]) + + self.bootstrap_pxc.assert_not_called() + self.service_restart.assert_called_once() + + def test_nochange_bootstrap_leader(self): + self.is_leader.return_value = True + self.is_leader_bootstrapped.return_value = False + + hooks.render_config_restart_on_changed([]) + + self.bootstrap_pxc.assert_called_once() + self.service_restart.assert_not_called() + + def test_nochange_bootstrapped_leader(self): + self.is_leader.return_value = True + self.is_leader_bootstrapped.return_value = True + + hooks.render_config_restart_on_changed([]) + + self.bootstrap_pxc.assert_not_called() + self.service_restart.assert_not_called() + + def test_nochange_noleader(self): + self.is_leader.return_value = False + self.is_leader_bootstrapped.return_value = False + + hooks.render_config_restart_on_changed([]) + + self.bootstrap_pxc.assert_not_called() + self.service_restart.assert_not_called() + + class TestClusterRelation(CharmTestCase): def setUp(self):