ServerConnector with default #selector & acceptors causes DOA jetty on high core count machines

[jtnord]

Jetty version

9.4.6 -> 9.4.25

Java version

various flavours of jdk8 (oracle, opendjk)

OS type/version

Windows and Linux

Description

When using Jetty with a ServerConnector created without specifying the number of acceptors and selectors Jetty does not process any incoming requests.

This has been observed reliably on a c5.4xlarge in AWS.

see jenkinsci/jenkins-test-harness#193 for an example.

The symptom of this is that Jetty starts and is listening (as shown by netstat) but any requests just timeout.

Note: the versions affected range is a tad unreliable. The code works with jetty-9.4.5.v20170502 and fails with later versions from maven central - but recompiling later jetty versions from a tag with jdk8 appears to resolve the issue (for an unknown reason) - the running JVM is the same in all cases. So I do not believe that code diff between jetty-9.4.5.v2017050 and jetty-9.4.5.v20180619 is of any use in order to bisect this issue.

[joakime]

What is "a high core count machine" to you?

We regularly run / build / test / load-test on 64 core machines. (default configurations of Jetty)
We also know of several 128 core machines running Jetty just fine.

According to https://aws.amazon.com/ec2/instance-types/c5/ the c5.4xlarge is only a 16 core machine.

[joakime]

Defaults for 9.4.25

Acceptors

https://github.com/eclipse/jetty.project/blob/a9729c7e7f33a459d2616a8f9e9ba8a90f432e95/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java#L222-L227

Between 1 and 4 if undeclared.

Selectors

https://github.com/eclipse/jetty.project/blob/a9729c7e7f33a459d2616a8f9e9ba8a90f432e95/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java#L71-L80

Between 1 and whatever is smaller (Core Count / 2) or (ThreadPool.maxThreads / 16)

Defaults for 9.4.5

Acceptors

https://github.com/eclipse/jetty.project/blob/9d8ad7489c2dc546e6cfe74c801d17f638430d1d/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java#L192-L197

Between 1 and 4 depending on core count.

Selectors

https://github.com/eclipse/jetty.project/blob/9d8ad7489c2dc546e6cfe74c801d17f638430d1d/jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java#L222-L223

Between 1 and 4 depending on core count.

[MarkEWaite]

The failures I was seeing were on a physical machine with 72 cores, not a virtualized machine. I was running an automated test for the platform labeler plugin. Same failures were detected with the git client plugin and the git plugin.

The issue I was seeing was resolved in all three cases by the change by @jglick .

[jtnord]

high is relative. Normally in our CI we use lower core count machines or containers and did not see this issue. However we did see this issue our windows CI which used 2 containers inside a windows server 2019 in (which was running in a c5.4xlarge).

I could not reproduce it on my windows laptop (just 4 cores - 8 including hyperthreading), or in smaller AWS machines. Running directly in windows server 2019 on a c5.4xlarge allowed us to observe this issue at will.

You could change the title to Jetty does not service requests with.... on c5.4xlarge

I can provide steps to reproduce if you require.

[sbordet]

Did you get any warning on startup from the thread pool budget?
How did you configure your thread pool (min and max)?

[jglick]

To reiterate jenkinsci/jenkins-test-harness#193 (comment), we are not talking here about Jetty performing a bit less than optimally under some heavy production workloads. The situation here is that we were creating a server using defaults

Server server = new Server(new ThreadPoolImpl(new ThreadPoolExecutor(10, 10, 10L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),new ThreadFactory() {…})));
// …
new ServerConnector(server)

and, in some environments (but not, say, on my laptop!) Jetty would completely fail to respond to a single request. Just sat there with the socket open. No visibly busy threads (just an idle-looking thread pool), no warnings, nothing. Adding , 1, 1 fixes the problem.

[sbordet]

@jglick the thread pool configuration is wrong.

You have at most 10 threads in the pool which is an incredibly small number for a server, also considering that Jetty steals some for the selectors, the acceptors, etc.

Please consider A) using Jetty QueuedThreadPool as it outperforms the JDK pools, and keep the max threads to a sane number (if in doubts, leave at default).

With only 10 threads, and Jetty using few internally, you probably are out of threads, so that your pool has none to actually serve requests.
Had you used Jetty's QueuedThreadPool you would have seen thread budget warnings at startup.

Is there any reason to underconfigure the thread pool in such a way?

[jglick]

an incredibly small number for a server

Again this is just a test service which rarely serves more than one concurrent connection. I am working on verifying whether a stock thread pool also avoids the issue.

[jtnord]

making the following change did indeed resolve the issue

-        server = new Server(new ThreadPoolImpl(new ThreadPoolExecutor(10, 10, 10L, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),new ThreadFactory() {
-            public Thread newThread(Runnable r) {
-                Thread t = new Thread(r);
-                t.setName("Jetty Thread Pool");
-                return t;
-            }
-        })));
+        server = new Server();

Just checked the javadoc for Server and it is non obvious that the selectors/acceptors steal from the pool.

I would suggest leaving this open to

1. produce a warning if #acceptors & selectors > threads in the pool
2. update the docs to say some threads will be stolen?

[sbordet]

@jtnord we do produce a warning for case 1. - the problem is that we can produce a warning only if we know the max number of threads of the thread pool.

We have Jetty's QueuedThreadPool and ExecutorThreadPool that support this warning mechanism.

But your code is using something different (ThreadPoolImpl) so we are out of luck.
Hence the suggestion to use Jetty classes or stick with all defaults like you have found out.

About 2. I filed #4493.

Thanks!

[joakime]

On thing that comes up often on stackoverflow regarding Jetty threading is the mistaken assumption that the ThreadPool is only used for request processing.

That's an incorrect assumption, the ThreadPool is generic, and can be used for anything that needs a thread within Jetty.
Your choice of technology can even impact your ThreadPool requirements (such as HTTP/2, or Async Servlets, or WebSocket, which all put a greater demand on the ThreadPool).

[joakime]

Also of note, the QueuedThreadPool supports ThreadFactory now (if you want to continue to control the names like your old code did)

See https://www.eclipse.org/jetty/javadoc/current/org/eclipse/jetty/util/thread/QueuedThreadPool.html#%3Cinit%3E(int,int,int,int,java.util.concurrent.BlockingQueue,java.lang.ThreadGroup,java.util.concurrent.ThreadFactory)

[jtnord]

Also of note, the QueuedThreadPool supports ThreadFactory now (if you want to continue to control the names like your old code did)

Thanks @joakime , however that gets us back to having hard coded numbers for min/maxt etc as you can only set the ThreadFactory if you also pass all other arguments (defaults are inlined), and I think we have decided to just let Jetty use its defaults as we don't know better (as proven)

[sbordet]

however that gets us back to having hard coded numbers for min/max etc as you can only set the ThreadFactory if you also pass all other arguments

The default values are "hardcoded" too in the sense that QueuedThreadPool won't dynamically change the value of maxThreads.

Your usage of ThreadFactory only sets the thread name (and all of them to the same string - not recommended because you won't know what thread did what in the logs, for example).
This can be done better by using QueuedThreadPool.setName(String) - no need for ThreadFactory.

I would recommend that you stick with QueuedThreadPool, set the name to have the threads named like you want, and choose a maxThreads that is large-ish for the job - the default is 200 threads max.

To sum up:

QueuedThreadPool pool = new QueuedThreadPool(/*maxThreads - this is optional*/);
pool.setName("Jetty Thread Pool");
server = new Server(pool);

If you really want complete control over the thread name:

QueuedThreadPool pool = new QueuedThreadPool() {
  @Override
  public Thread newThread(Runnable job) {
    // Create thread.
  }
};

However, note that QueuedThreadPool.newThread(Runnable) does more than just setting the name, so be careful if you don't call super.

[jtnord]

Your usage of ThreadFactory only sets the thread name (and all of them to the same string - not recommended because you won't know what thread did what in the logs, for example).

we do not count on a thread name in logs for correlation - given that can change at runtime. If you are doing that I recommend you switch to logging the thread id :)

Strangely enough at least the acceptor thread does change the thread name?
"Jetty Thread Pool-acceptor-0@63cf0f08-ServerConnector@7309e547{HTTP/1.1,[http/1.1]}{localhost:59622}" #17 daemon prio=3 os_prio=-1 tid=0x000000001ebbd800 nid=0x4b18 runnable [0x00000000242ee000]

Thank you for the reference to QueuedThreadPool.setThreadName() I will switch to that.

[sbordet]

Jetty does change the thread names, for example the thread serving a request will have the request URI in its name.

Typically that's ok as name-counter or tid are used to grep the logs and those are preserved when the name is changed.

@jtnord feel free to close the issue if that's solved for you.

[stale]

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been closed due to it having no activity.