[ https://issues.apache.org/jira/browse/HIVE-2446?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13105710#comment-13105710 ]
jirapos...@reviews.apache.org commented on HIVE-2446: ----------------------------------------------------- ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1909/#review1918 ----------------------------------------------------------- trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java <https://reviews.apache.org/r/1909/#comment4399> should -1 be replaced by 0 here? trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java <https://reviews.apache.org/r/1909/#comment4400> I've thought it over again and feel it's better to use the counter "name" here rather than "display name". Display name has the benefit that the users see the same name at the JT page and the metrics page, but display name could be too long (sometimes may contain special characters) and it could be changed over time. I think "name" should be a better ID here. The only caveat is that the person who's looking at the metrics need to know how it is translated to the display name in JT page. trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java <https://reviews.apache.org/r/1909/#comment4398> Is this line too long? Hive's line length shouldn't exceed 100 chars. trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/ClientStatsPublisher.java <https://reviews.apache.org/r/1909/#comment4401> Rather than passing JobID, it would be more flexible to pass a String converted from jobID. This is the approach that StatsPublisher interface is using. - Ning On 2011-09-14 22:19:21, Robert Surówka wrote: bq. bq. ----------------------------------------------------------- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1909/ bq. ----------------------------------------------------------- bq. bq. (Updated 2011-09-14 22:19:21) bq. bq. bq. Review request for hive and Ning Zhang. bq. bq. bq. Summary bq. ------- bq. bq. The purpose of this change is to allow publication or storage of counters while the job is running. bq. bq. Introduced two new variables to hive-default.xml and HiveConf.java: "hive.client.stats.publishers" and "hive.client.stats.counters". First one specifies classes names, whose instances will be executed by HadoopJobExecHelper.java (similarly as hooks are) in its method progress(ExecDriverTaskHandle): MapRedStats. Second one specifies list of counters that any client stat publishers should publish or stored. Details regarding format of this list is up to a specific deployment (it is up to client stats publishers to parse it), yet it is required to use display names of counter groups and counters. bq. bq. Added interface ClientStatsPublishers in org.apache.hadoop.hive.ql.stats package, that must be implemented by all stats publishers. bq. bq. Added code to progress(ExecDriverTaskHandle): MapRedStats from HadoopJobExecHelper.java that puts counters' values to a Java map and then executes registered client stats publishers giving them that map and running job id. Added two new methods to HadoopJobExecHelper: extractAllCounterValues(Counters) and getClientStatsPublishers() that are used by code from previous sentence. bq. bq. Made cosmetic changes in two other classes bq. bq. bq. This addresses bug HIVE-2446. bq. https://issues.apache.org/jira/browse/HIVE-2446 bq. bq. bq. Diffs bq. ----- bq. bq. trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1170867 bq. trunk/conf/hive-default.xml 1170867 bq. trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestMarkPartition.java 1170867 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java 1170867 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java 1170867 bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/ClientStatsPublisher.java PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/1909/diff bq. bq. bq. Testing bq. ------- bq. bq. Run some random tests, and still running the unit tests. bq. bq. bq. Thanks, bq. bq. Robert bq. bq. > Introduction of client statistics publishers possibility > -------------------------------------------------------- > > Key: HIVE-2446 > URL: https://issues.apache.org/jira/browse/HIVE-2446 > Project: Hive > Issue Type: Improvement > Components: Clients, Statistics > Reporter: Robert Surówka > Priority: Minor > Attachments: HIVE-2446.1.patch, HIVE-2446.1.patch > > Original Estimate: 1h > Remaining Estimate: 1h > > The purpose of this change is to allow publication or storage of counters > while the job is running. > Introduced two new variables to hive-default.xml and HiveConf.java: > "hive.client.stats.publishers" and "hive.client.stats.counters". First one > specifies classes names, whose instances will be executed by > HadoopJobExecHelper.java (similarly as hooks are) in its method > progress(ExecDriverTaskHandle): MapRedStats. Second one specifies list of > counters that any client stat publishers should publish or stored. Details > regarding format of this list is up to a specific deployment (it is up to > client stats publishers to parse it), yet it is required to use display names > of counter groups and counters. > Added interface ClientStatsPublishers in org.apache.hadoop.hive.ql.stats > package, that must be implemented by all stats publishers. > Added code to progress(ExecDriverTaskHandle): MapRedStats from > HadoopJobExecHelper.java that puts counters' values to a Java map and then > executes registered client stats publishers giving them that map and running > job id. Added two new methods to HadoopJobExecHelper: > extractAllCounterValues(Counters) and getClientStatsPublishers() that are > used by code from previous sentence. > Made cosmetic changes in two other classes > -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira