----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52029/#review149468 -----------------------------------------------------------
Just nits. Nice clean code. Thanks! common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java (line 209) <https://reviews.apache.org/r/52029/#comment217113> nit: Too long line (100 char) common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java (line 117) <https://reviews.apache.org/r/52029/#comment217114> nit: Too long line (100 char) common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricVariableRatioGauge.java (line 24) <https://reviews.apache.org/r/52029/#comment217115> nit: extra line? common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricVariableRatioGauge.java (line 33) <https://reviews.apache.org/r/52029/#comment217116> nit: indent by 4 space instead common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java (line 53) <https://reviews.apache.org/r/52029/#comment217118> nit: indent by 4 space instead common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestMetricVariableRatioGauge.java (line 67) <https://reviews.apache.org/r/52029/#comment217119> nit: maybe one test, where the ration is not decimal? service/src/java/org/apache/hive/service/cli/session/SessionManager.java (line 260) <https://reviews.apache.org/r/52029/#comment217122> Documentation - we should create a documentation of these statistics. This abandoned session metrics was not clear for me - had to read the code :) service/src/test/org/apache/hive/service/cli/session/TestSessionManagerMetrics.java (line 152) <https://reviews.apache.org/r/52029/#comment217124> nit: Too long line (100 char) service/src/test/org/apache/hive/service/cli/session/TestSessionManagerMetrics.java (line 154) <https://reviews.apache.org/r/52029/#comment217125> nit: Too long line (100 char) service/src/test/org/apache/hive/service/cli/session/TestSessionManagerMetrics.java (line 252) <https://reviews.apache.org/r/52029/#comment217126> nit: name (testAciveSessionTimeMetrics) - Peter Vary On Sept. 19, 2016, 1:21 p.m., Barna Zsombor Klara wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52029/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2016, 1:21 p.m.) > > > Review request for hive, Gabor Szadovszky, Peter Vary, and Sergio Pena. > > > Repository: hive-git > > > Description > ------- > > HIVE-14753: Track the number of open/closed/abandoned sessions in HS2 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java > 9be9b50aa02ff88816eb92079eaff9afa3e1be90 > common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java > 4297233ed12a7d9a2fa03ac3204e8335c0aed821 > > common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsConstant.java > 9dc96f9c6412720a891b5c55e2074049c893d780 > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java > 4c433678bd62ea74b80babce9856681192deb25f > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/MetricVariableRatioGauge.java > PRE-CREATION > common/src/test/org/apache/hadoop/hive/common/metrics/MetricsTestUtils.java > 46676589e6656d0f13f1931bfe67a63dd1920042 > > common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestMetricVariableRatioGauge.java > PRE-CREATION > service/src/java/org/apache/hive/service/cli/session/SessionManager.java > 15bab0660fcb9a997d66f6ff0a5dbc0e39c37ae7 > > service/src/test/org/apache/hive/service/cli/session/TestSessionManagerMetrics.java > 5511c54ff431211f7f72deaa017c915b839dfb2a > > Diff: https://reviews.apache.org/r/52029/diff/ > > > Testing > ------- > > Ran the unit tests in the common and the ql subprojects. > Manually verified the metrics using the HS2 webui metric dump. > > > Thanks, > > Barna Zsombor Klara > >
