Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 9.4.x 3550 queued thread pool stalled #3586

Merged
merged 5 commits into from Apr 25, 2019

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Apr 24, 2019

This fixes #3550 by ensure that a new thread is always started when:

  • a task is executed, the queue is not empty and there are no idle threads available (this may race, but will only start addition threads)
  • if a thread exits and the queue is not empty

gregw and others added 3 commits April 24, 2019 09:33
Fixed QueuedThreadPool dump of known threads

Signed-off-by: Greg Wilkins <gregw@webtide.com>
previously if there were no idle threads, QueuedThreadPool.execute()
would just queue the job and not start a new thread to run it

Signed-off-by: lachan-roberts <lachlan@webtide.com>
Ensure that new threads are started if a thread exits due to failure.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw added Bug For general bugs on Jetty side High Priority Sponsored This issue affects a user with a commercial support agreement labels Apr 24, 2019
QueuedThreadPool pool = new QueuedThreadPool(4, 3);

String dump = pool.dump();
// TODO use hamcrest 2.0 regex matcher
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to do a refresh of maven-test-helper for Jetty 9.4.x anyway.
In the meantime you can use the maven-test-helper regex version ...

import static org.eclipse.jetty.websocket.common.test.MoreMatchers.regex;

assertThat(endpoint.capture.pollMessages(), regex("^onBinary\\(.*InputStream.*"));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never mind, that's a websocket specific matcher.
i'll upgrade maven-test-helper today. (and even include the new Net helper for junit 5. 879e161)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with these changes.

@@ -603,7 +604,7 @@ public void dump(Appendable out, String indent) throws IOException
String knownMethod = "";
for (StackTraceElement t : trace)
{
if ("idleJobPoll".equals(t.getMethodName()) && t.getClassName().endsWith("QueuedThreadPool"))
if ("idleJobPoll".equals(t.getMethodName()) && t.getClassName().endsWith("QueuedThreadPool$Runner"))
Copy link
Contributor

@joakime joakime Apr 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be ...

if ("idleJobPoll".equals(t.getMethodName()) && t.getClassName().equals(Runner.class.getName()))

instead? To avoid static name of class, if someone refactors that class (again), which broken this before.

@gregw gregw merged commit a3f6d3a into jetty-9.4.x Apr 25, 2019
@gregw gregw deleted the jetty-9.4.x-3550-QueuedThreadPool-stalled branch April 25, 2019 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side High Priority Sponsored This issue affects a user with a commercial support agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server becomes unresponsive after sitting idle from a load spike
3 participants