You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/10/21 05:12:41 UTC

[GitHub] [tomcat] anilgursel opened a new pull request #371: Call `accept` immediately if connection count is not close to max con…

anilgursel opened a new pull request #371:
URL: https://github.com/apache/tomcat/pull/371

…nections

Under regular load, we observe NIO2 has better CPU utilization and slightly better latency compared to NIO1. However, under heavy load or during spikes, we observed that the 99th percentile latency of NIO2 is significantly worse compared to NIO1.

- We see that the latency is worse compared to NIO1 only for the requests that end up establishing a new connection.
- When `maxKeepAliveRequests=-1`, we do not observe the latency degradation with NIO2 at all. But, we need and use Tomcat’s `maxKeepAliveRequests` feature, so setting it to `-1` is not an option.
- The problem occurs only during spikes/high load, e.g., maxThreads=40, while the concurrent users sending requests is 100.
- The issue happens only for blocking scenarios, where Tomcat threads are blocked during request processing. When non-blocking programming is used (where Tomcat threads are freed quickly), we do not see any degradation compared to NIO1. However, we have many applications that would not be able to adopt non-blocking anytime soon.

While looking at Tomcat code, we saw that the `accept` is scheduled to be run on the thread pool. Our hypothesis was that this scheduling was causing the `accept` to be delayed significantly under heavy load. So, for a request with new connection establishment, it needs to go through the `TaskQueue` twice. So, just for testing purpose we set `maxConnections=-1` to see the impact of immediate `accept` call (and in our test the number of concurrent connections does not get close to `maxConnections`), we observed that the 99th percentile latency problem with NIO2 disappeared.

Accordingly, proposing a change to call `accept` immediately if the number of connections are not close to `maxConnections`.

----------------------------------------------------------------
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.

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] anilgursel commented on pull request #371: NIO2: Call `accept` immediately if connection count is not close to max con…

Posted by GitBox <gi...@apache.org>.

anilgursel commented on pull request #371:
URL: https://github.com/apache/tomcat/pull/371#issuecomment-718799285


   I updated the PR to have it as `getMaxConnections() / 2`.  I agree it is just a heuristic and not clean.  We can potentially make it configurable?
   
   If my understanding is correct, one reason to schedule the `accept` is to prevent blocking the thread.  The reason behind blocking (via `LimitLatch`) is to have a hard limit on the `maxConnections`.  Is there any possibility to relax this a bit?  Something like:
   
   ```java
   else if (getConnectionCount() < getMaxConnections()) {
       increment();
       serverSock.accept(null, this);
   }
   
   ```
   
   With this, there is possibility of number of connections to go over `maxConnections` little bit (which I think the expectation can be managed in documentation).  Actually, I believe there will only be one thread doing `accept` logic at any given time.  Because, a new `accept` is scheduled only when the previous one is `completed`.   Is this true?  If so,  it would not go beyond maxConnections.


----------------------------------------------------------------
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.

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] rmaucher commented on pull request #371: NIO2: Call `accept` immediately if connection count is not close to max con…

Posted by GitBox <gi...@apache.org>.

rmaucher commented on pull request #371:
URL: https://github.com/apache/tomcat/pull/371#issuecomment-718632321

maxConnections overall is a bad design for NIO2, that is a bit obvious. It started with connections not being counted accurately then after some iterative refactorings of close it remains a bad idea. For the longest time the default was fixed to -1 (still there in 8.5).

I guess all you're doing is add a heuristic that works for you [but actually no guarantee]. Also about accept, I refactored it NIO2 style to save one thread and it seems more efficient overall, before it was a much more classical NIO1 style accept using the same dedicated thread (so one extra thread, and likely less efficient). The downside of the strategy is that the accept is just like the other processing threads when maxConnections is used. If there's a heavy thread starvation, then accept will get delayed. I don't think it's the end of the world, the server will actually be slow for everyone, but I understand. OTOH, if your app is as you say using non blocking IO and is saving on threads, then there's no downside to the strategy [even with full use of the thread pool, the threads become available very frequently and accept stays responsive].

So I'm ok with adding some heuristic, but maxThreads can be too low. Maybe maxConnections/2 would be good enough, with another heuristic on a minimal number ... Not very clean ...

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] rmaucher commented on pull request #371: NIO2: Call `accept` immediately if connection count is not close to max con…

Posted by GitBox <gi...@apache.org>.

rmaucher commented on pull request #371:
URL: https://github.com/apache/tomcat/pull/371#issuecomment-718885690


   Hum, ok, since incrementing is not done in parallel, then it's something that can be tried.


----------------------------------------------------------------
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.

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