Merge ~ballot/content-cache-charm/+git/content-cache-charm:worker_connections into content-cache-charm:master

Proposed by Benjamin Allot
Status: Work in progress
Proposed branch: ~ballot/content-cache-charm/+git/content-cache-charm:worker_connections
Merge into: content-cache-charm:master
Prerequisite: ~ballot/content-cache-charm/+git/content-cache-charm:reuseport
Diff against target: 130 lines (+31/-5)
5 files modified
config.yaml (+2/-1)
lib/haproxy.py (+3/-2)
reactive/content_cache.py (+12/-0)
tests/unit/test_content_cache.py (+8/-2)
tests/unit/test_nginx.py (+6/-0)
Reviewer Review Type Date Requested Status
Content Cache Charmers Pending
Review via email: mp+400075@code.launchpad.net
To post a comment you must log in.

Unmerged commits

95f319d... by Benjamin Allot

WIP

cbdb480... by Benjamin Allot

Add the reuseport directive in nginx.

According to several resources, the way Linux handles TCP connection
with epoll is responsible for the way the load is balanced between the
workers.

This is an option turned off by default for now.

See [0] and [1] for reference.

[0]: https://blog.cloudflare.com/the-sad-state-of-linux-socket-balancing/
[1]: https://www.nginx.com/blog/performance-tuning-tips-tricks/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/config.yaml b/config.yaml
2index 3ce504a..c843b93 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -132,11 +132,12 @@ options:
6 description: >
7 Override default ciphers used for TLS/SSL termination (OpenSSL format).
8 worker_connections:
9- default: 768
10+ default: 0
11 type: int
12 description: >
13 Sets the maximum number of simultaneous connections that can be
14 opened by an Nginx worker process.
15+ Defaults to auto-calculate (0), which is (number of sites * max_connections / worker_processes).
16 worker_processes:
17 default: 0
18 type: int
19diff --git a/lib/haproxy.py b/lib/haproxy.py
20index 5153d97..814bc89 100644
21--- a/lib/haproxy.py
22+++ b/lib/haproxy.py
23@@ -339,7 +339,8 @@ backend backend-{name}
24
25 return rendered_output
26
27- def _calculate_num_procs_threads(self, num_procs, num_threads):
28+ @classmethod
29+ def calculate_num_procs_threads(self, num_procs, num_threads):
30 if num_procs and num_threads:
31 ver = utils.package_version('haproxy')
32 # With HAProxy 2, nbproc and nbthreads are mutually exclusive.
33@@ -359,7 +360,7 @@ backend backend-{name}
34 return (num_procs, num_threads)
35
36 def render(self, config, num_procs=None, num_threads=None, monitoring_password=None, tls_cipher_suites=None):
37- (num_procs, num_threads) = self._calculate_num_procs_threads(num_procs, num_threads)
38+ (num_procs, num_threads) = self.calculate_num_procs_threads(num_procs, num_threads)
39
40 listen_stanzas = self.render_stanza_listen(config)
41
42diff --git a/reactive/content_cache.py b/reactive/content_cache.py
43index 15cf56d..5cc47df 100644
44--- a/reactive/content_cache.py
45+++ b/reactive/content_cache.py
46@@ -7,6 +7,7 @@ import time
47 from copy import deepcopy
48
49 import jinja2
50+import multiprocessing
51 import yaml
52
53 from charms import reactive
54@@ -199,6 +200,17 @@ def configure_nginx(conf_path=None):
55
56 connections = config['worker_connections']
57 processes = config['worker_processes']
58+ # Auto-calculate worker_connections
59+ nb_sites = len(sites.keys())
60+ (haproxy_proc, haproxy_thread) = HAProxy.calculate_num_procs_threads(None, None)
61+ if config['max_connections'] == 0:
62+ haproxy_connections
63+ if processes == 0:
64+ nb_processes = multiprocessing.cpu_count()
65+ else:
66+ nb_processes = processes
67+ if connections == 0:
68+ connections = nb_sites * haproxy_max_connections / nb_processes
69 if ngx_conf.sync_sites(sites.keys()) or ngx_conf.set_workers(connections, processes):
70 hookenv.log('Enabled sites: {}'.format(' '.join(sites.keys())))
71 changed = True
72diff --git a/tests/unit/test_content_cache.py b/tests/unit/test_content_cache.py
73index b8de692..59afe21 100644
74--- a/tests/unit/test_content_cache.py
75+++ b/tests/unit/test_content_cache.py
76@@ -1423,10 +1423,12 @@ site1.local:
77 'enable_cache_background_update': False,
78 'enable_cache_lock': False,
79 'enable_prometheus_metrics': True,
80+ 'max_connections': 0,
81 'metrics_listen_address': '127.0.0.2',
82 'reuseport': True,
83 'sites': ngx_config,
84- 'worker_connections': 768,
85+ # 'worker_connections': 768,
86+ 'worker_connections': 0,
87 'worker_processes': 0,
88 }
89
90@@ -1437,7 +1439,8 @@ site1.local:
91 os.mkdir(os.path.join(self.tmpdir, 'conf.d'))
92 os.mkdir(os.path.join(self.tmpdir, 'sites-available'))
93 os.mkdir(os.path.join(self.tmpdir, 'sites-enabled'))
94- shutil.copyfile('tests/unit/files/nginx.conf', os.path.join(self.tmpdir, 'nginx.conf'))
95+ nginx_conf = os.path.join(self.tmpdir, 'nginx.conf')
96+ shutil.copyfile('tests/unit/files/nginx.conf', nginx_conf)
97
98 opened_ports.return_value = ['80/tcp', '{0}/tcp'.format(nginx.METRICS_PORT)]
99 content_cache.configure_nginx(self.tmpdir)
100@@ -1451,3 +1454,6 @@ site1.local:
101 with open(test_file, 'r', encoding='utf-8') as f:
102 got = f.read()
103 self.assertEqual(got, want)
104+ with open(nginx_conf, 'r', encoding='utf-8') as f:
105+ self.assertRegex(f.read(), r'.*worker_connections = '
106+ assert nginx
107diff --git a/tests/unit/test_nginx.py b/tests/unit/test_nginx.py
108index 2734123..6e6f209 100644
109--- a/tests/unit/test_nginx.py
110+++ b/tests/unit/test_nginx.py
111@@ -4,6 +4,8 @@ import sys
112 import tempfile
113 import unittest
114
115+from unittest import mock
116+
117 import jinja2
118 import yaml
119
120@@ -20,6 +22,10 @@ class TestLibNginx(unittest.TestCase):
121 self.tmpdir = tempfile.mkdtemp(prefix='charm-unittests-')
122 self.addCleanup(shutil.rmtree, self.tmpdir)
123 self.charm_dir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))
124+ patcher = mock.patch('multiprocessing.cpu_count')
125+ self.mock_cpu_count = patcher.start()
126+ self.addCleanup(patcher.stop)
127+ self.mock_cpu_count.return_value = 4
128
129 def test_nginx_config_conf_path(self):
130 conf_path = '/etc/nginx/conf.d'

Subscribers

People subscribed via source and target branches