----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52084/#review149805 -----------------------------------------------------------
Thanks Zsombor! Godd refactor, just minor nits, and some questions. Thanks, Peter common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java (line 262) <https://reviews.apache.org/r/52084/#comment217526> nit: retrieving common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java (line 281) <https://reviews.apache.org/r/52084/#comment217527> nit: retrieving common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java (line 318) <https://reviews.apache.org/r/52084/#comment217528> nit: retrieving common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java <https://reviews.apache.org/r/52084/#comment217529> Not really into this part of the code, but we might want at least check that calling the startStoredScope again will not cause any trouble here. Would be perfect if we could check the log, but that might be an overkill :) metastore/src/java/org/apache/hadoop/hive/metastore/HMSMetricsListener.java (lines 48 - 50) <https://reviews.apache.org/r/52084/#comment217530> If refactoring, this 3 lines might be put into an util method, to avoid code duplication :). Not that important, just an idea - Peter Vary On Sept. 21, 2016, 9:24 a.m., Barna Zsombor Klara wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52084/ > ----------------------------------------------------------- > > (Updated Sept. 21, 2016, 9:24 a.m.) > > > Review request for hive, Gabor Szadovszky, Peter Vary, Sergio Pena, and > Szehon Ho. > > > Repository: hive-git > > > Description > ------- > > I have refactored the exception handling in the metrics API. The initial > objective was to remove the throws clause from the methods which did not > throw IOExceptions but were declaring it. Finally I continued to remove the > throws clause from the rest of the APIs since we were never able to do > anything else with the excpetions except log them out. New log messages were > introduced inside the metrics classes to capture exceptional conditions so we > can omit the try catch blocks when creating/updating metrics. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java > 9be9b50aa02ff88816eb92079eaff9afa3e1be90 > common/src/java/org/apache/hadoop/hive/common/metrics/MetricsMBean.java > 19946d9e93d55bbffb6d03cbc35e569849a86dd8 > common/src/java/org/apache/hadoop/hive/common/metrics/MetricsMBeanImpl.java > a973155f079c6124a6981b04123d9496dc5d3448 > common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java > 4297233ed12a7d9a2fa03ac3204e8335c0aed821 > > common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java > 4c433678bd62ea74b80babce9856681192deb25f > common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java > 6a5d22f2bcc0a7efd869f7c0c0c8fad33ca35f74 > > common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java > a3fb04f1ab9be8be9f69c616eabeb534b2a2d560 > metastore/src/java/org/apache/hadoop/hive/metastore/HMSMetricsListener.java > 6830cf75dc163230cdabb4ed41d65c222c6ca54d > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > f0b84768e891e4281a613e68590524646a038435 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java > 42d398dcc9a37bdb2d90c940c579c55fb76b4cca > service/src/java/org/apache/hive/service/cli/operation/Operation.java > 90fe76d00e6f833e18d183c290f13c23db9303a1 > > Diff: https://reviews.apache.org/r/52084/diff/ > > > Testing > ------- > > Tested basic metric gathering with both codahale and legacy metrics class. > Updated and ran unit tests in the common project. > > > Thanks, > > Barna Zsombor Klara > >
