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

[ZEPPELIN-3978] Change Jetty Server to use QueuedThreadPool and make it configurable #3294

Conversation

Projects
None yet
4 participants
@fred521
Copy link

commented Jan 30, 2019

What is this PR for?

By default Jetty Server only supports 200 threads.
On our daily use cases, we normally have more than 100 users on the single Zeppelin Server, which will create more than 200 threads in Jetty.

That could cause slowness or stuck because users could not receive WebSocket message.

Add configurable param feature to scale up the performance for Single Zeppelin Server whenever needs it

What type of PR is it?

Improvement

Todos

  • - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-3978
[ZEPPELIN-3978]

How should this be tested?

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@felixcheung felixcheung changed the title Main stream/fdai increase thread pool number [ZEPPELIN-3978] Main stream/fdai increase thread pool number Jan 30, 2019

@felixcheung

This comment has been minimized.

Copy link
Member

commented Jan 30, 2019

can you take a look at the test failures? https://travis-ci.org/fred521/zeppelin/builds/486180306

@felixcheung

This comment has been minimized.

Copy link
Member

commented Jan 30, 2019

can you update the PR title? should it say "Change Jetty to use threadpool and make it configurable"

@zjffdu

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2019

Thanks @fred521 Could you add these configuration into zeppelin-site.xml.template ?

@fred521 fred521 changed the title [ZEPPELIN-3978] Main stream/fdai increase thread pool number [ZEPPELIN-3978] Change Jetty Server to use Queue Thread Pool and make it configurable Feb 1, 2019

@fred521 fred521 changed the title [ZEPPELIN-3978] Change Jetty Server to use Queue Thread Pool and make it configurable [ZEPPELIN-3978] Change Jetty Server to use QueuedThreadPool and make it configurable Feb 1, 2019

@fred521

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

can you update the PR title? should it say "Change Jetty to use threadpool and make it configurable"

Yes, I updated, thanks

@fred521

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

Thanks @fred521 Could you add these configuration into zeppelin-site.xml.template ?

yep, updated it, thanks

@fred521

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

can you take a look at the test failures? https://travis-ci.org/fred521/zeppelin/builds/486180306

checked it and re-run for a few times, each time actually taking for 30mins to 1 hour.
My code changes shouldn't break any unit test, I guess there are configurations not match to the VM box.

And I feel that will stop a lot of contributors, can Apache Zeppelin Community improve this?

Otherwise, new contributors will feel wasting time to spend effort here.

cc @Leemoonsoo @zjffdu

@Leemoonsoo

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

#3296 probably helps. @zjffdu right?

@zjffdu

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2019

@fred521 I think ZEPPELIN-3957 solve this issue, Could you do a rebase ? And I create another ticket to improve it as well (ZEPPELIN-3983)

fdai added some commits Jan 25, 2019

fdai
custom the thread number to have better performance
Reviewers: dac, nlu, nwang, huijunw, yaoli, hluo, mfu, yaliangw

Differential Revision: https://phabricator.twitter.biz/D265510

@fred521 fred521 force-pushed the fred521:mainStream/fdai_increase_thread_pool_number branch from 6c5f277 to 62986a2 Feb 2, 2019

@fred521

This comment has been minimized.

Copy link
Author

commented Feb 2, 2019

@fred521 I think ZEPPELIN-3957 solve this issue, Could you do a rebase ? And I create another ticket to improve it as well (ZEPPELIN-3983)

Yeah, thanks, finnnnnnally, passed all

@zjffdu

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2019

Awesome, thanks @fred521 LGTM

@asfgit asfgit closed this in 0d3ac8a Feb 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.