You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "babyblue94520 (via GitHub)" <gi...@apache.org> on 2023/08/09 02:52:59 UTC

[GitHub] [tomcat] babyblue94520 opened a new pull request, #645: feat(Http11Processor): Add max value in keepalive.

babyblue94520 opened a new pull request, #645:
URL: https://github.com/apache/tomcat/pull/645

   __HttpClient__ judges how many connections are cached by the __keepalive__ header max value, but __Http11Processor__ doesn't provide it, so __HttpClient__ __keepAliveConnections__ is always __5__.
   
   ## sun.net.www.http.HttpClient
   
   Parse keepalive headers for max connections.
   
   * __parseHTTPHeader__
           
   ```java
   HeaderParser p = new HeaderParser(responses.findValue("Keep-Alive"));
   /* default should be larger in case of proxy */
   keepAliveConnections = p.findInt("max", usingProxy?50:5);
   keepAliveTimeout = p.findInt("timeout", usingProxy?60:5);
   ```
       
    * __finished__
   
   ```java
   public void finished() {
       if (reuse) /* will be reused */
           return;
       keepAliveConnections--;
       poster = null;
       if (keepAliveConnections > 0 && isKeepingAlive() &&
              !(serverOutput.checkError())) {
           /* This connection is keepingAlive && still valid.
            * Return it to the cache.
            */
           putInKeepAliveCache();
       } else {
           closeServer();
       }
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] babyblue94520 closed pull request #645: feat(Http11Processor): Add max value in keepalive.

Posted by "babyblue94520 (via GitHub)" <gi...@apache.org>.
babyblue94520 closed pull request #645: feat(Http11Processor): Add max value in keepalive.
URL: https://github.com/apache/tomcat/pull/645


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] aooohan commented on a diff in pull request #645: feat(Http11Processor): Add max value in keepalive.

Posted by "aooohan (via GitHub)" <gi...@apache.org>.
aooohan commented on code in PR #645:
URL: https://github.com/apache/tomcat/pull/645#discussion_r1289481686


##########
java/org/apache/coyote/http11/Http11Processor.java:
##########
@@ -999,7 +999,12 @@ protected final void prepareResponse() throws IOException {
                     int keepAliveTimeout = protocol.getKeepAliveTimeout();
 
                     if (keepAliveTimeout > 0) {
-                        String value = "timeout=" + keepAliveTimeout / 1000L;
+                        StringBuilder value = new StringBuilder();
+                        value.append("timeout=").append(keepAliveTimeout / 1000L);
+                        int maxKeepAliveRequests = protocol.getMaxKeepAliveRequests();
+                        if (maxKeepAliveRequests > 0) {

Review Comment:
   > If maxKeepAliveRequests is `-1` then set `max` to Integer.MAX_VALUE
   
   I haven't found any RFC documents about how to response this parameter if no limit on server.
   I think personally that if we don't limit the number of request on the connection. We should ignore this parameter, and leave it up to the client to decide how to handle this connection (e.g. use their default strategy). In additional, that's how **httpd** handles it if I read correct.
   
   https://github.com/apache/httpd/blob/d62d3143f42771a7111c2148837c0a91a271b83d/modules/http/http_protocol.c#L287-L299



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] markt-asf commented on a diff in pull request #645: feat(Http11Processor): Add max value in keepalive.

Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on code in PR #645:
URL: https://github.com/apache/tomcat/pull/645#discussion_r1288767872


##########
java/org/apache/coyote/http11/Http11Processor.java:
##########
@@ -999,7 +999,12 @@ protected final void prepareResponse() throws IOException {
                     int keepAliveTimeout = protocol.getKeepAliveTimeout();
 
                     if (keepAliveTimeout > 0) {
-                        String value = "timeout=" + keepAliveTimeout / 1000L;
+                        StringBuilder value = new StringBuilder();
+                        value.append("timeout=").append(keepAliveTimeout / 1000L);
+                        int maxKeepAliveRequests = protocol.getMaxKeepAliveRequests();
+                        if (maxKeepAliveRequests > 0) {

Review Comment:
   If maxKeepAliveRequests is `-1` then set `max` to Integer.MAX_VALUE



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[GitHub] [tomcat] babyblue94520 commented on a diff in pull request #645: feat(Http11Processor): Add max value in keepalive.

Posted by "babyblue94520 (via GitHub)" <gi...@apache.org>.
babyblue94520 commented on code in PR #645:
URL: https://github.com/apache/tomcat/pull/645#discussion_r1289412320


##########
java/org/apache/coyote/http11/Http11Processor.java:
##########
@@ -999,7 +999,12 @@ protected final void prepareResponse() throws IOException {
                     int keepAliveTimeout = protocol.getKeepAliveTimeout();
 
                     if (keepAliveTimeout > 0) {
-                        String value = "timeout=" + keepAliveTimeout / 1000L;
+                        StringBuilder value = new StringBuilder();
+                        value.append("timeout=").append(keepAliveTimeout / 1000L);
+                        int maxKeepAliveRequests = protocol.getMaxKeepAliveRequests();
+                        if (maxKeepAliveRequests > 0) {

Review Comment:
   How is this idea implemented here? Because it affects keepAliveLeft.
   
   [AbstractEndpoint](https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/util/net/AbstractEndpoint.java)
   ```
   public int getMaxKeepAliveRequests() {
       // Disable keep-alive if the server socket is not bound
       if (bindState.isBound()) {
           return maxKeepAliveRequests;
       } else {
           return 1;
       }
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org