diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h index 6fbe0f36599..db30be7cf64 100644 --- a/proxy/http/HttpTransact.h +++ b/proxy/http/HttpTransact.h @@ -722,10 +722,15 @@ class HttpTransact 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 @@ class HttpTransact 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 00000000000..ff5b5495cef --- /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 d1d4a4da614..380fc928f09 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 @@ def __init__(self) -> None: 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 @@ def run(self) -> None: """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 @@ def run(self) -> None: 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()