ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jaimin Jetly" <jai...@hortonworks.com>
Subject Re: Review Request 41997: Upgrading Ambari Jetty Server from Jetty 8 to Jetty 9
Date Thu, 14 Jan 2016 20:46:45 GMT


> On Jan. 12, 2016, 11:42 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java,
line 420
> > <https://reviews.apache.org/r/41997/diff/3/?file=1194891#file1194891line420>
> >
> >     This is a problem - using Jetty's calculation on systems with 64+ CPUs can cause
thread starvation issues. We should use the calculation we had before.
> 
> Jaimin Jetly wrote:
>     Even before, we did not have any calculation for setting selector count and relied
on Jetty's calculation. If I am missing something over here please point me to a github link
of the present ambari code that sets selector count. 
>     I will be happy to fix the selector count issue irrespective of it being an existing
issue or being introduced as part of this patch. 
>     I just need to know what is the logic to calculate the desired selector count
> 
> Jonathan Hurley wrote:
>     So, in Jetty 7, we let Jetty create the number of acceptor and selector threads that
it wanted to. Take the case of a machine with 48 processors. Jetty's internal code would have
calculated the number of acceptors at 12, the number of selectors at 12. Our default pool
size was 25 threads, leaving only a single thread to answer actual requests. 
>     
>     https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java#L506-L508
>     The above code ensures that our pool size is large enough to ensure there are available
threads beyond those that Jetty reserves. This way, we let Jetty determine what it wants and
just ensure that the thread pool is large enough.
>     
>     Now that's talking about API connections. For the agent connections, things are a
little different. There are two connectors sharing the same port binding. So, we halve the
acceptors. However this code change passes in 0 for the selectors. We should let Jetty calculate
what it wants, and then cut it in half since we're using two connectors. Passing in 0 could
create too many selectors compared to the number of acceptors. We should do what we're doing
above and just adjust thread pool size if necessary, and pass in 1/2 the calculated selectors/acceptors.

I have uploaded new revision of the patch for the review that cuts default selector count
of Jetty in half as well. 

Although I wanted to clarify following:

>> Take the case of a machine with 48 processors. Jetty's internal code would have calculated
the number of acceptors at 12, the number of selectors at 12. Our default pool size was 25
threads, leaving only a single thread to answer actual requests.

This is not true with respect to Jetty 9 internal code. http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-server/9.2.2.v20140723/org/eclipse/jetty/server/AbstractConnector.java#189
and http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-server/9.2.2.v20140723/org/eclipse/jetty/server/ServerConnector.java#209


Jetty-9 code formula for default acceptor count: Math.max(1, Math.min(4, processors/8))
Jetty-9 code formula for default selector count: Math.max(1, Math.min(4, processors/2)))

This means that irrespective of number of processors, Jetty-9 default calculation will never
allocate more than 4 acceptor or selector threads , and so should ideally not cause starvation
of wroker threads in QTP.

By adding explicit logic of doing halves to the selector/acceptor count: Math.max(2, Default
count/ 2), Ambari for now with Jetty-9 server is fixing the value of selector/acceptor threads
to 2. As Default acceptos/selector count can at max be 4, by doing its half we reduce it to
2 and the formula in that case becomes Math.max(2, 2)


- Jaimin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41997/#review114107
-----------------------------------------------------------


On Jan. 14, 2016, 8:29 p.m., Jaimin Jetly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41997/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2016, 8:29 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley and Yusaku Sako.
> 
> 
> Bugs: AMBARI-14231
>     https://issues.apache.org/jira/browse/AMBARI-14231
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The reviewboard addresses the concerns raised at https://reviews.apache.org/r/41018/
and fixes the unit test failures that happened due to the original patch uploaded at https://reviews.apache.org/r/41018/
> 
> 
> Diffs
> -----
> 
>   ambari-funtest/pom.xml d84b1c8 
>   ambari-project/pom.xml 68bdb94 
>   ambari-server/pom.xml c0010fb 
>   ambari-server/src/main/java/org/apache/ambari/server/api/AmbariErrorHandler.java c4a80f2

>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 353a972

>   ambari-server/src/test/java/org/apache/ambari/server/api/AmbariErrorHandlerTest.java
8b4f99c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariHandlerListTest.java
2e2cc45 
>   ambari-web/pom.xml d160e37 
> 
> Diff: https://reviews.apache.org/r/41997/diff/
> 
> 
> Testing
> -------
> 
> 1. Ambari Jetty server is being upgraded to 9.2.11 version. Verifed that there is no
known critcial vulnerability with this version of Jetty (http://www.eclipse.org/jetty/documentation/current/security-reports.html)
> 2. ambari-web resources files are no longer explicitly compressed in the maven build
process with the changes in the patch. Verified manually by using browser developer tools
that ambari-server does this dynamically for any *.js and *.css files.
> 3. Verified manually that HDP cluster and views can be created with the changes made
in the patch.
> 4. Verified that all functional tests written in ambari-funtest project passes with the
changes done in the patch.
> 5. Verified that all unit tests passes with the changes done in the patch.
> 
> #mvn clean test:#
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 approved: 41
licence.
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Ambari Main ........................................ SUCCESS [  6.024 s]
> [INFO] Apache Ambari Project POM .......................... SUCCESS [  0.053 s]
> [INFO] Ambari Web ......................................... SUCCESS [01:01 min]
> [INFO] Ambari Views ....................................... SUCCESS [  4.496 s]
> [INFO] Ambari Admin View .................................. SUCCESS [ 13.839 s]
> [INFO] ambari-metrics ..................................... SUCCESS [  0.439 s]
> [INFO] Ambari Metrics Common .............................. SUCCESS [  3.800 s]
> [INFO] Ambari Metrics Hadoop Sink ......................... SUCCESS [  6.071 s]
> [INFO] Ambari Metrics Flume Sink .......................... SUCCESS [  4.089 s]
> [INFO] Ambari Metrics Kafka Sink .......................... SUCCESS [  5.205 s]
> [INFO] Ambari Metrics Storm Sink .......................... SUCCESS [  1.802 s]
> [INFO] Ambari Metrics Collector ........................... SUCCESS [01:31 min]
> [INFO] Ambari Metrics Monitor ............................. SUCCESS [  4.857 s]
> [INFO] Ambari Metrics Assembly ............................ SUCCESS [  6.001 s]
> [INFO] Ambari Server ...................................... SUCCESS [  01:19 h]
> [INFO] Ambari Functional Tests ............................ SUCCESS [01:17 min]
> [INFO] Ambari Agent ....................................... SUCCESS [ 16.568 s]
> [INFO] Ambari Client ...................................... SUCCESS [  0.041 s]
> [INFO] Ambari Python Client ............................... SUCCESS [  1.307 s]
> [INFO] Ambari Groovy Client ............................... SUCCESS [ 13.300 s]
> [INFO] Ambari Shell ....................................... SUCCESS [  0.046 s]
> [INFO] Ambari Python Shell ................................ SUCCESS [  0.086 s]
> [INFO] Ambari Groovy Shell ................................ SUCCESS [ 10.767 s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 01:25 h
> [INFO] Finished at: 2016-01-06T14:49:30-08:00
> [INFO] Final Memory: 90M/606M
> 
> 
> Thanks,
> 
> Jaimin Jetly
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message