----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51698/#review148070 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java (line 377) <https://reviews.apache.org/r/51698/#comment215474> Unncessary comments. ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java (line 50) <https://reviews.apache.org/r/51698/#comment215475> We should try configuration for supplied hostname, if not found then switch to InetAddress, which can be unreliable or not what user wants. ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DBSessionEventListener.java (line 66) <https://reviews.apache.org/r/51698/#comment215477> Please define and enum or EnumSet and put comments. ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DBSessionEventListener.java (line 137) <https://reviews.apache.org/r/51698/#comment215479> This is not a threadsafe check. Necessary to declare isInint as volatile. Best is to do the init outside this method call. ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DbMetricSource.java (line 34) <https://reviews.apache.org/r/51698/#comment215486> If this instance is created by guice, why the private constructor? ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DbMetricSource.java (line 35) <https://reviews.apache.org/r/51698/#comment215481> Can you rename to DBMetricsSource, since interface is named MetricsSource (plural) ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DbMetricSource.java (line 43) <https://reviews.apache.org/r/51698/#comment215487> Not the optimium impl of Singleton patter if you have have to use it, static reference instantiation is recommeneded. ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java (line 51) <https://reviews.apache.org/r/51698/#comment215488> Again improper use of Singleton. Why is guice not used ? - Sid Wagle On Sept. 7, 2016, 5:59 p.m., Li-Wei Tseng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51698/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2016, 5:59 p.m.) > > > Review request for Ambari and Aravindan Vijayan. > > > Bugs: AMBARI-17589 > https://issues.apache.org/jira/browse/AMBARI-17589 > > > Repository: ambari > > > Description > ------- > > Added Database metrics generated by Eclipselink Performance Profiler > > > Diffs > ----- > > ambari-server/conf/unix/metrics.properties 5f01e39 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > d20b1d7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > bd34c25 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/AmbariMetricSink.java > 809176be > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java > 23845c9 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSink.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java > cf10408 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java > 58e2045 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java > 4618618 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/Configuration.java > 705971f > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DBSessionEventListener.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DbMetricSource.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java > a04ca43 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java > 1645ebf > > ambari-server/src/main/java/org/apache/ambari/server/orm/EclipseLinkSessionCustomizer.java > 6717e01 > ambari-server/src/main/resources/META-INF/persistence.xml 2bc3282 > > ambari-server/src/test/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImplTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/51698/diff/ > > > Testing > ------- > > Unit test for emit metrics to AMS > > > Thanks, > > Li-Wei Tseng > >