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