> On Jan. 6, 2017, 7:12 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, > > line 109 > > <https://reviews.apache.org/r/55278/diff/2/?file=1599025#file1599025line109> > > > > How do the internal workings of metricsSource.publish(...) work? If > > it's synchronous in any way, this could cause a massive performance issue. > > > > If it simply queues data to be pushed to Ambari Metrics and returns > > immediately, then I think we're OK and you can drop the issue. > > > > I'm just wanting to make sure we're not sneaking in a major bottleneck. > > Aravindan Vijayan wrote: > Jonathan, it is entirely asynchronous. Even pre-processing before sending > to AMS is done asynchronously.
Great - I hadn't made the connection that the source below (which enqueues runnables) is what makes this async. > On Jan. 6, 2017, 7:12 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java, > > lines 106-114 > > <https://reviews.apache.org/r/55278/diff/2/?file=1599025#file1599025line106> > > > > This kind of seems like a workflow problem, no? We shouldn't need to > > keep track of whether it was initialized. > > Aravindan Vijayan wrote: > The AmbariPerformanceMonitor is designed to work with the > DatabaseMetricsSource to send metrics to AMS. Since the > AmbariPerformanceMonitor is instantiated through EclipseLink, and > DatabaseMetricsSource is instantiated explicitly through MetricsService > initialization, the Monitor might come up before the Source. Hence, I added > this initialization check & retry. > > One can still use the inbuilt PeformanceMonitor to look at metrics > through logs if not interested in visualizing them through AMS. Hmmm - that kind of stinks, but I see what you mean. However, when you call init() you're missing metrics, right? Since you never re-submit it. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55278/#review160792 ----------------------------------------------------------- On Jan. 6, 2017, 5:44 p.m., Aravindan Vijayan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55278/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2017, 5:44 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Dmytro Sen, Jonathan Hurley, > Sumit Mohanty, and Sid Wagle. > > > Bugs: AMBARI-17596 > https://issues.apache.org/jira/browse/AMBARI-17596 > > > Repository: ambari > > > Description > ------- > > Extend the eclipse link PeformanceMonitor utility to collect and send metrics > to AMS. > Counter, Timer and Avg time (Timer/Counter) metrics are tracked. > > The metrics.properties file has enough information to know how to configure > the Database metrics source to collect metrics of interest. > > > Diffs > ----- > > ambari-server/conf/unix/metrics.properties 3ee22d6 > ambari-server/conf/windows/metrics.properties 3ee22d6 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsService.java > 4a613f0 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/MetricsSource.java > 400dcb6 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AbstractMetricsSource.java > 6bdd0ba > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariMetricSinkImpl.java > be24988 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/AmbariPerformanceMonitor.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/DatabaseMetricsSource.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/JvmMetricsSource.java > cb9f275 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsConfiguration.java > ca83a53 > > ambari-server/src/main/java/org/apache/ambari/server/metrics/system/impl/MetricsServiceImpl.java > d0d2e69 > > ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/JvmMetricsSourceTest.java > 9f649b4 > > ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsServiceTest.java > 4029f25 > > ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestAmbariMetricsSinkImpl.java > 3565504 > > ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/TestMetricsSource.java > acf1586 > ambari-server/src/test/resources/metrics.properties 5eee064 > > Diff: https://reviews.apache.org/r/55278/diff/ > > > Testing > ------- > > Manually tested. > Unit tests added. > > > File Attachments > ---------------- > > Sample Grafana dashboard for AmbariServer DB metrics > > https://reviews.apache.org/media/uploaded/files/2017/01/06/a6f88cb6-743f-44ed-a851-3e959072b176__Screen_Shot_2017-01-06_at_12.59.00_PM.png > > > Thanks, > > Aravindan Vijayan > >
