----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48030/#review136400 -----------------------------------------------------------
Ship it! Ship It! - Sriharsha Chintalapani On May 30, 2016, 8:21 a.m., Jungtaek Lim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48030/ > ----------------------------------------------------------- > > (Updated May 30, 2016, 8:21 a.m.) > > > Review request for Ambari, Sriharsha Chintalapani and Sid Wagle. > > > Bugs: AMBARI-16946 > https://issues.apache.org/jira/browse/AMBARI-16946 > > > Repository: ambari > > > Description > ------- > > There's a mismatch between TimelineMetricsCache and Storm metrics unit, while > TimelineMetricsCache considers "metric name + timestamp" to be unique but > Storm is not. > > For example, assume that bolt B has task T1, T2 and B has registered metrics > M1. It's possible for metrics sink to receive (T1, M1) and (T2, M1) with same > timestamp TS1 (in TaskInfo, not current time), and received later will be > discarded from TimelineMetricsCache. > > If we want to have unique metric point of Storm, we should use "topology name > + component name + task id + metric name" to metric name so that "metric name > + timestamp" will be unique. > > There're other issues I would like to address, too. > > - Currently, hostname is written to hostname of the machine which runs > metrics sink. Since TaskInfo has hostname of the machine which runs task, > we're better to use this. > - Unit of timestamp of TaskInfo is second, while Storm Metrics Sink uses this > as millisecond, resulting in timestamp flaw, and malfunction of cache > eviction. It should be multiplied by 1000. > - 'component name' is not unique across the cluster, so it's not fit for app > id. 'topology name' is unique so proper value of app id is topology name. > > Consideration: Hostname for determining 'write shard' is set to hostname of > the machine which runs metrics sink. Since I don't know read also be sharded, > I'm not sure it's safe to use TaskInfo's hostname to hostname of > TimelineMetric. Please review carefully regarding this. > > > Diffs > ----- > > > ambari-metrics/ambari-metrics-storm-sink/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSink.java > 02f5598 > > ambari-metrics/ambari-metrics-storm-sink/src/test/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSinkTest.java > 8171a4d > > Diff: https://reviews.apache.org/r/48030/diff/ > > > Testing > ------- > > I tested this only ambari-metrics module since changeset is not related on > other modules. > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] ambari-metrics ..................................... SUCCESS [ 0.896 > s] > [INFO] Ambari Metrics Common .............................. SUCCESS [ 13.386 > s] > [INFO] Ambari Metrics Hadoop Sink ......................... SUCCESS [ 5.085 > s] > [INFO] Ambari Metrics Flume Sink .......................... SUCCESS [ 6.827 > s] > [INFO] Ambari Metrics Kafka Sink .......................... SUCCESS [ 4.190 > s] > [INFO] Ambari Metrics Storm Sink .......................... SUCCESS [ 1.384 > s] > [INFO] Ambari Metrics Collector ........................... SUCCESS [04:06 > min] > [INFO] Ambari Metrics Monitor ............................. SUCCESS [ 3.556 > s] > [INFO] Ambari Metrics Grafana ............................. SUCCESS [01:03 > min] > [INFO] Ambari Metrics Assembly ............................ SUCCESS [ 3.567 > s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 05:48 min > [INFO] Finished at: 2016-05-30T16:46:07+09:00 > [INFO] Final Memory: 78M/1038M > [INFO] > ------------------------------------------------------------------------ > > In fact, I tried to run `mvn test` from ambari root directory but build is > failing from ambari-web. > > ``` > > [email protected] install > > /Users/jlim/WorkArea/JavaProjects/ambari/ambari-web/node_modules/chokidar/node_modules/fsevents > > node-gyp rebuild > ... > npm WARN install:fsevents [email protected] install: `node-gyp rebuild` > npm WARN install:fsevents Exit status 1 > ``` > > No luck on `npm install`, too. > > > Thanks, > > Jungtaek Lim > >
