From d25ab7bd130b4c58bcbf2fec8d01f43f9ac1a884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9rome=20Perrin?= <jerome@nexedi.com> Date: Mon, 13 Jun 2022 22:22:42 +0900 Subject: [PATCH] software/erp5: set server timeout in haproxy to match publisher-timeout Instead of having an hardcoded timeout that users will hit anyway even if they increase publisher timeout, set this timeout value to be slightly higher than publisher timeout. This way publisher-timeout can be used to allow longer requests and it's generally more consistent. --- software/erp5/test/test/test_balancer.py | 40 ++++++++++++++++++++++-- stack/erp5/buildout.hash.cfg | 6 ++-- stack/erp5/haproxy.cfg.in | 14 +++++++-- stack/erp5/instance-balancer.cfg.in | 4 +-- stack/erp5/instance-erp5.cfg.in | 3 ++ 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/software/erp5/test/test/test_balancer.py b/software/erp5/test/test/test_balancer.py index 53ca774de..0d3097000 100644 --- a/software/erp5/test/test/test_balancer.py +++ b/software/erp5/test/test/test_balancer.py @@ -170,6 +170,7 @@ class BalancerTestCase(ERP5InstanceTestCase): 'ssl': { 'caucase-url': cls.getManagedResource("caucase", CaucaseService).url, }, + 'timeout-dict': {}, 'family-path-routing-dict': {}, 'path-routing-list': [], } @@ -186,18 +187,51 @@ class BalancerTestCase(ERP5InstanceTestCase): class SlowHTTPServer(ManagedHTTPServer): - """An HTTP Server which reply after 2 seconds. + """An HTTP Server which reply after a timeout. + + Timeout is 2 seconds by default, and can be specified in the path of the URL """ class RequestHandler(BaseHTTPRequestHandler): def do_GET(self): # type: () -> None self.send_response(200) self.send_header("Content-Type", "text/plain") - time.sleep(2) + timeout = 2 + try: + timeout = int(self.path[1:]) + except ValueError: + pass + time.sleep(timeout) self.end_headers() self.wfile.write(b"OK\n") - log_message = logging.getLogger(__name__ + '.SlowHandler').info + log_message = logging.getLogger(__name__ + '.SlowHTTPServer').info + + +class TestTimeout(BalancerTestCase, CrontabMixin): + __partition_reference__ = 't' + @classmethod + def _getInstanceParameterDict(cls): + # type: () -> dict + parameter_dict = super(TestTimeout, cls)._getInstanceParameterDict() + # use a slow server instead + parameter_dict['dummy_http_server'] = [[cls.getManagedResource("slow_web_server", SlowHTTPServer).netloc, 1, False]] + # and set timeout of 1 second + parameter_dict['timeout-dict'] = {'default': 1} + return parameter_dict + + def test_timeout(self): + # type: () -> None + self.assertEqual( + requests.get( + six.moves.urllib.parse.urljoin(self.default_balancer_url, '/1'), + verify=False).status_code, + requests.codes.ok) + self.assertEqual( + requests.get( + six.moves.urllib.parse.urljoin(self.default_balancer_url, '/5'), + verify=False).status_code, + requests.codes.gateway_timeout) class TestLog(BalancerTestCase, CrontabMixin): diff --git a/stack/erp5/buildout.hash.cfg b/stack/erp5/buildout.hash.cfg index ea6f4e51b..204a0a5c4 100644 --- a/stack/erp5/buildout.hash.cfg +++ b/stack/erp5/buildout.hash.cfg @@ -74,7 +74,7 @@ md5sum = 9a7f7888ba4183c9d900e862074f3baf [template-erp5] filename = instance-erp5.cfg.in -md5sum = fcecee930aa05a1a739f31b28f6e4769 +md5sum = 3d8f3a440b7423c3b947c6ea4d775c6e [template-zeo] filename = instance-zeo.cfg.in @@ -90,11 +90,11 @@ md5sum = c3e3f8cd985407931b705d15bdedc8d9 [template-balancer] filename = instance-balancer.cfg.in -md5sum = 4491b256241cb4bbdfafb22ae95a3eba +md5sum = d51d718ab85ae57726e6f0bf81c4bfeb [template-haproxy-cfg] filename = haproxy.cfg.in -md5sum = d2d98ed3fafce764991b72371e3e09d5 +md5sum = 1645ef8990ab2b50f91a4c02f0cf8882 [template-rsyslogd-cfg] filename = rsyslogd.cfg.in diff --git a/stack/erp5/haproxy.cfg.in b/stack/erp5/haproxy.cfg.in index d5b1b3c1d..c3e0bc70e 100644 --- a/stack/erp5/haproxy.cfg.in +++ b/stack/erp5/haproxy.cfg.in @@ -48,6 +48,7 @@ # ( 8000, # port int # 'https', # proto str # True, # ssl_required bool + # None, # timeout (in seconds) int | None # [ # backends # '10.0.0.10:8001', # netloc str # 1, # max_connection_count int @@ -59,6 +60,7 @@ # ( 8002, # port int # 'https', # proto str # False, # ssl_required bool + # None, # timeout (in seconds) int | None # [ # backends # '10.0.0.10:8003', # netloc str # 1, # max_connection_count int @@ -135,7 +137,6 @@ defaults timeout connect 10s timeout queue 60s - timeout server 305s timeout client 305s option http-server-close @@ -150,7 +151,7 @@ defaults {% set family_path_routing_dict = parameter_dict['family-path-routing-dict'] %} {% set path_routing_list = parameter_dict['path-routing-list'] %} -{% for name, (port, _, certificate_authentication, backend_list) in sorted(six.iteritems(parameter_dict['backend-dict'])) -%} +{% for name, (port, _, certificate_authentication, timeout, backend_list) in sorted(six.iteritems(parameter_dict['backend-dict'])) -%} listen family_{{ name }} {%- if parameter_dict.get('ca-cert') -%} {%- set ssl_auth = ' ca-file ' ~ parameter_dict['ca-cert'] ~ ' verify' ~ ( ' required' if certificate_authentication else ' optional' ) ~ ' crl-file ' ~ parameter_dict['crl'] %} @@ -162,6 +163,15 @@ listen family_{{ name }} cookie SERVERID rewrite http-request set-header X-Balancer-Current-Cookie SERVERID +{% if timeout %} + {# + Apply a slightly longer timeout than the zope timeout so that clients can see the + TimeoutReachedError from zope, that is a bit more informative than the 504 error + page from haproxy. + #} + timeout server {{ timeout + 3 }}s +{%- endif %} + # remove X-Forwarded-For unless client presented a verified certificate acl client_cert_verified ssl_c_used ssl_c_verify 0 http-request del-header X-Forwarded-For unless client_cert_verified diff --git a/stack/erp5/instance-balancer.cfg.in b/stack/erp5/instance-balancer.cfg.in index 341b31e12..55ceb253f 100644 --- a/stack/erp5/instance-balancer.cfg.in +++ b/stack/erp5/instance-balancer.cfg.in @@ -177,7 +177,7 @@ update-command = ${:command} {% else %} {% set external_scheme = 'https' -%} {% endif -%} -{% do haproxy_dict.__setitem__(family_name, (haproxy_port, external_scheme, slapparameter_dict['ssl-authentication-dict'].get(family_name, False), zope_family_address_list)) -%} +{% do haproxy_dict.__setitem__(family_name, (haproxy_port, external_scheme, slapparameter_dict['ssl-authentication-dict'].get(family_name, False), slapparameter_dict['timeout-dict'].get(family_name, None), zope_family_address_list)) -%} {% endfor -%} [haproxy-cfg-parameter-dict] @@ -309,7 +309,7 @@ config-port = {{ next(six.itervalues(haproxy_dict))[0] }} [{{ section('publish') }}] recipe = slapos.cookbook:publish.serialised -{% for family_name, (port, scheme, _, _) in haproxy_dict.items() -%} +{% for family_name, (port, scheme, _, _, _) in haproxy_dict.items() -%} {{ family_name ~ '-v6' }} = {% if ipv6_set %}{{ scheme ~ '://[' ~ ipv6 ~ ']:' ~ port }}{% endif %} {{ family_name }} = {{ scheme ~ '://' ~ ipv4 ~ ':' ~ port }} {% endfor -%} diff --git a/stack/erp5/instance-erp5.cfg.in b/stack/erp5/instance-erp5.cfg.in index f351e0161..0d1bdc6fe 100644 --- a/stack/erp5/instance-erp5.cfg.in +++ b/stack/erp5/instance-erp5.cfg.in @@ -271,6 +271,7 @@ software-type = zope {% set zope_family_name_list = [] -%} {% set zope_backend_path_dict = {} -%} {% set ssl_authentication_dict = {} -%} +{% set balancer_timeout_dict = {} -%} {% set jupyter_zope_family_default = [] -%} {% for custom_name, zope_parameter_dict in six.iteritems(zope_partition_dict) -%} {% set partition_name = 'zope-' ~ custom_name -%} @@ -288,6 +289,7 @@ software-type = zope {% do zope_backend_path_dict.__setitem__(zope_family, backend_path) -%} {% do ssl_authentication_dict.__setitem__(zope_family, zope_parameter_dict.get('ssl-authentication', False)) -%} {% set current_zope_family_override_dict = zope_family_override_dict.get(zope_family, {}) -%} +{% do balancer_timeout_dict.__setitem__(zope_family, current_zope_family_override_dict.get('publisher-timeout', global_publisher_timeout)) -%} [{{ section_name }}] <= request-zope-base name = {{ partition_name }} @@ -406,6 +408,7 @@ config-url = ${request-jupyter:connection-url} 'zope-family-dict': zope_family_parameter_dict, 'backend-path-dict': zope_backend_path_dict, 'ssl-authentication-dict': ssl_authentication_dict, + 'timeout-dict': balancer_timeout_dict, 'apachedex-promise-threshold': monitor_dict.get('apachedex-promise-threshold', 70), 'apachedex-configuration': monitor_dict.get( 'apachedex-configuration', -- 2.30.9