This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new a4f2f3d05 per_server.connection.max: CONNECT requests (#9182)
a4f2f3d05 is described below
commit a4f2f3d05e2a5fba370d6abd6eef936f63b8eef0
Author: Brian Neradt <[email protected]>
AuthorDate: Mon Nov 7 21:22:17 2022 -0600
per_server.connection.max: CONNECT requests (#9182)
We were putting servers that hit the per_server.connection.max limit on
the dead server list if the client were connecting to them via HTTP
CONNECT requests. This fixes the handling of these so the server is not
put on the dead server list.
---
proxy/http/HttpTransact.h | 21 ++++--
.../origin_connection/gold/two_503_congested.gold | 9 +++
.../per_server_connection_max.test.py | 88 ++++++++++++++++++++--
3 files changed, 107 insertions(+), 11 deletions(-)
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 6fbe0f365..db30be7cf 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -722,10 +722,15 @@ public:
int64_t internal_msg_buffer_size = 0; // out
int64_t internal_msg_buffer_fast_allocator_size = -1;
- int scheme = -1; // out
- int next_hop_scheme = scheme; // out
- int orig_scheme = scheme; // pre-mapped scheme
- int method = 0;
+ int scheme = -1; // out
+ int next_hop_scheme = scheme; // out
+ int orig_scheme = scheme; // pre-mapped scheme
+ int method = 0;
+
+ /// The errno associated with a failed connect attempt.
+ ///
+ /// This is used for logging and (in some code paths) for determing HTTP
+ /// response reason phrases.
int cause_of_death_errno = -UNKNOWN_INTERNAL_ERROR; // in
ink_time_t client_request_time = UNDEFINED_TIME; // internal
@@ -918,7 +923,13 @@ public:
void
set_connect_fail(int e)
{
- if (e == EIO || this->current.server->connect_result == EIO) {
+ if (e == EUSERS) {
+ // EUSERS is used when the number of connections exceeds the configured
+ // limit. Since this is not a network connectivity issue with the
+ // server, we should not mark it as such. Otherwise we will incorrectly
+ // mark the server as down.
+ this->current.server->connect_result = 0;
+ } else if (e == EIO || this->current.server->connect_result == EIO) {
this->current.server->connect_result = e;
}
if (e != EIO) {
diff --git a/tests/gold_tests/origin_connection/gold/two_503_congested.gold
b/tests/gold_tests/origin_connection/gold/two_503_congested.gold
new file mode 100644
index 000000000..ff5b5495c
--- /dev/null
+++ b/tests/gold_tests/origin_connection/gold/two_503_congested.gold
@@ -0,0 +1,9 @@
+``
+> CONNECT``
+``
+< HTTP/1.1 503 Origin server congested
+``
+> CONNECT``
+``
+< HTTP/1.1 503 Origin server congested
+``
diff --git
a/tests/gold_tests/origin_connection/per_server_connection_max.test.py
b/tests/gold_tests/origin_connection/per_server_connection_max.test.py
index d1d4a4da6..380fc928f 100644
--- a/tests/gold_tests/origin_connection/per_server_connection_max.test.py
+++ b/tests/gold_tests/origin_connection/per_server_connection_max.test.py
@@ -33,22 +33,23 @@ class PerServerConnectionMaxTest:
self._configure_server()
self._configure_trafficserver()
- def _configure_dns(self):
+ def _configure_dns(self) -> None:
"""Configure a nameserver for the test."""
- self._nameserver = Test.MakeDNServer("dns", default='127.0.0.1')
+ self._dns = Test.MakeDNServer("dns1", default='127.0.0.1')
def _configure_server(self) -> None:
"""Configure the server to be used in the test."""
- self._server = Test.MakeVerifierServerProcess('server',
self._replay_file)
+ self._server = Test.MakeVerifierServerProcess('server1',
self._replay_file)
def _configure_trafficserver(self) -> None:
"""Configure Traffic Server to be used in the test."""
- self._ts = Test.MakeATSProcess("ts")
+ self._ts = Test.MakeATSProcess("ts1")
self._ts.Disk.remap_config.AddLine(
f'map / http://127.0.0.1:{self._server.Variables.http_port}'
)
self._ts.Disk.records_config.update({
- 'proxy.config.dns.nameservers':
f"127.0.0.1:{self._nameserver.Variables.Port}",
+ 'proxy.config.dns.nameservers':
f"127.0.0.1:{self._dns.Variables.Port}",
+ 'proxy.config.dns.resolv_conf': 'NULL',
'proxy.config.diags.debug.enabled': 1,
'proxy.config.diags.debug.tags': 'http',
'proxy.config.http.per_server.connection.max':
self._origin_max_connections,
@@ -61,7 +62,7 @@ class PerServerConnectionMaxTest:
"""Configure the TestRun."""
tr = Test.AddTestRun(
'Verify we enforce proxy.config.http.per_server.connection.max')
- tr.Processes.Default.StartBefore(self._nameserver)
+ tr.Processes.Default.StartBefore(self._dns)
tr.Processes.Default.StartBefore(self._server)
tr.Processes.Default.StartBefore(self._ts)
@@ -71,4 +72,79 @@ class PerServerConnectionMaxTest:
http_ports=[self._ts.Variables.port])
+class ConnectMethodTest:
+ """Test our max origin connection behavior with CONNECT traffic."""
+
+ _client_counter: int = 0
+ _origin_max_connections: int = 3
+
+ def __init__(self) -> None:
+ """Configure the server processes in preparation for the TestRun."""
+ self._configure_dns()
+ self._configure_origin_server()
+ self._configure_trafficserver()
+
+ def _configure_dns(self) -> None:
+ """Configure a nameserver for the test."""
+ self._dns = Test.MakeDNServer("dns2", default='127.0.0.1')
+
+ def _configure_origin_server(self) -> None:
+ """Configure the httpbin origin server."""
+ self._server = Test.MakeHttpBinServer("server2")
+
+ def _configure_trafficserver(self) -> None:
+ self._ts = Test.MakeATSProcess("ts2")
+
+ self._ts.Disk.records_config.update({
+ 'proxy.config.dns.nameservers':
f"127.0.0.1:{self._dns.Variables.Port}",
+ 'proxy.config.dns.resolv_conf': 'NULL',
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http|dns|hostdb',
+ 'proxy.config.http.server_ports': f"{self._ts.Variables.port}",
+ 'proxy.config.http.connect_ports':
f"{self._server.Variables.Port}",
+
+ 'proxy.config.http.per_server.connection.max':
self._origin_max_connections,
+ })
+
+ self._ts.Disk.remap_config.AddLines([
+ f"map http://foo.com/
http://www.this.origin.com:{self._server.Variables.Port}/";,
+ ])
+
+ def _configure_client_with_slow_response(self, tr) -> 'Test.Process':
+ """Configure a client to perform a CONNECT request with a slow
response from the server."""
+ p = tr.Processes.Process(f'slow_client_{self._client_counter}')
+ self._client_counter += 1
+ p.Command = f"curl -v --fail -s -p -x
127.0.0.1:{self._ts.Variables.port} 'http://foo.com/delay/2'"
+ return p
+
+ def run(self) -> None:
+ """Verify per_server.connection.max with CONNECT traffic."""
+ tr = Test.AddTestRun()
+ tr.Processes.Default.StartBefore(self._dns)
+ tr.Processes.Default.StartBefore(self._server)
+ tr.Processes.Default.StartBefore(self._ts)
+
+ slow0 = self._configure_client_with_slow_response(tr)
+ slow1 = self._configure_client_with_slow_response(tr)
+ slow2 = self._configure_client_with_slow_response(tr)
+
+ tr.Processes.Default.StartBefore(slow0)
+ tr.Processes.Default.StartBefore(slow1)
+ tr.Processes.Default.StartBefore(slow2)
+
+ # With those three slow transactions going on in the background, do a
+ # couple quick transactions and make sure they both reply with a 503
+ # response.
+ tr.Processes.Default.Command = (
+ f"sleep 1; curl -v --fail -s -p -x
127.0.0.1:{self._ts.Variables.port} 'http://foo.com/get'"
+ f"--next -v --fail -s -p -x 127.0.0.1:{self._ts.Variables.port}
'http://foo.com/get'"
+ )
+ # Curl will have a 22 exit code if it receives a 5XX response (and we
+ # expect a 503).
+ tr.Processes.Default.ReturnCode = 22
+ tr.Processes.Default.Streams.stderr = "gold/two_503_congested.gold"
+ tr.Processes.Default.TimeOut = 3
+
+
PerServerConnectionMaxTest().run()
+ConnectMethodTest().run()
