per_server.connection.max: CONNECT requests

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) {

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
+``

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()