You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashutosh Chauhan <ha...@apache.org> on 2013/12/05 02:01:45 UTC
Review Request 16026: No need to aggregate statistics collected via counter
mechanism
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16026/
-----------------------------------------------------------
Review request for hive and Navis Ryu.
Bugs: HIVE-5916
https://issues.apache.org/jira/browse/HIVE-5916
Repository: hive
Description
-------
No need to aggregate statistics collected via counter mechanism
Diffs
-----
trunk/itests/qtest/pom.xml 1547966
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 1547966
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java 1547966
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 1547966
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1547966
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java 1547966
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1547966
trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/CounterStatsAggregator.java 1547966
trunk/ql/src/test/queries/clientpositive/stats_counter_partitioned.q PRE-CREATION
trunk/ql/src/test/results/clientpositive/stats12.q.out 1547966
trunk/ql/src/test/results/clientpositive/stats13.q.out 1547966
trunk/ql/src/test/results/clientpositive/stats_counter_partitioned.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/16026/diff/
Testing
-------
Added a new test case. Ran all other stats test.
Thanks,
Ashutosh Chauhan
Re: Review Request 16026: No need to aggregate statistics collected via
counter mechanism
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16026/
-----------------------------------------------------------
(Updated Dec. 5, 2013, 5:06 a.m.)
Review request for hive and Navis Ryu.
Changes
-------
Addressed comments.
Bugs: HIVE-5916
https://issues.apache.org/jira/browse/HIVE-5916
Repository: hive
Description
-------
No need to aggregate statistics collected via counter mechanism
Diffs (updated)
-----
trunk/itests/qtest/pom.xml 1548014
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 1548014
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java 1548014
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 1548014
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1548014
trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java 1548014
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1548014
trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/CounterStatsAggregator.java 1548014
trunk/ql/src/test/queries/clientpositive/stats_counter_partitioned.q PRE-CREATION
trunk/ql/src/test/results/clientpositive/stats12.q.out 1548014
trunk/ql/src/test/results/clientpositive/stats13.q.out 1548014
trunk/ql/src/test/results/clientpositive/stats_counter_partitioned.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/16026/diff/
Testing
-------
Added a new test case. Ran all other stats test.
Thanks,
Ashutosh Chauhan
Re: Review Request 16026: No need to aggregate statistics collected via
counter mechanism
Posted by Ashutosh Chauhan <ha...@apache.org>.
> On Dec. 5, 2013, 3:59 a.m., Navis Ryu wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java, line 978
> > <https://reviews.apache.org/r/16026/diff/1/?file=394331#file394331line978>
> >
> > instead of specifying "statsPublisher instanceof CounterStatsPublisher", how about to introduce new marker interface for publisher/aggregator something like AccumulatedStatProvider or something? Just a suggestion.
I think that will be overkill. We already have 4 different ways of collecting stats, adding more interfaces will make it even more confusing.
> On Dec. 5, 2013, 3:59 a.m., Navis Ryu wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java, line 980
> > <https://reviews.apache.org/r/16026/diff/1/?file=394331#file394331line980>
> >
> > can have a length issue but will be fixed by HIVE-5936.
Yeah, could run into length issues though less likely since we now have shorter aggKey. Anyway, HIVE-5936 will take care of it.
> On Dec. 5, 2013, 3:59 a.m., Navis Ryu wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 8515
> > <https://reviews.apache.org/r/16026/diff/1/?file=394336#file394336line8515>
> >
> > comments might be fixed accordantly
will, update the comment.
> On Dec. 5, 2013, 3:59 a.m., Navis Ryu wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/CounterStatsAggregator.java, line 52
> > <https://reviews.apache.org/r/16026/diff/1/?file=394337#file394337line52>
> >
> > would it better to just adding ", e)" in the end?
will do.
- Ashutosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16026/#review29794
-----------------------------------------------------------
On Dec. 5, 2013, 1:01 a.m., Ashutosh Chauhan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16026/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2013, 1:01 a.m.)
>
>
> Review request for hive and Navis Ryu.
>
>
> Bugs: HIVE-5916
> https://issues.apache.org/jira/browse/HIVE-5916
>
>
> Repository: hive
>
>
> Description
> -------
>
> No need to aggregate statistics collected via counter mechanism
>
>
> Diffs
> -----
>
> trunk/itests/qtest/pom.xml 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/CounterStatsAggregator.java 1547966
> trunk/ql/src/test/queries/clientpositive/stats_counter_partitioned.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/stats12.q.out 1547966
> trunk/ql/src/test/results/clientpositive/stats13.q.out 1547966
> trunk/ql/src/test/results/clientpositive/stats_counter_partitioned.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16026/diff/
>
>
> Testing
> -------
>
> Added a new test case. Ran all other stats test.
>
>
> Thanks,
>
> Ashutosh Chauhan
>
>
Re: Review Request 16026: No need to aggregate statistics collected via
counter mechanism
Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16026/#review29794
-----------------------------------------------------------
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
<https://reviews.apache.org/r/16026/#comment57278>
instead of specifying "statsPublisher instanceof CounterStatsPublisher", how about to introduce new marker interface for publisher/aggregator something like AccumulatedStatProvider or something? Just a suggestion.
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
<https://reviews.apache.org/r/16026/#comment57279>
can have a length issue but will be fixed by HIVE-5936.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/16026/#comment57280>
comments might be fixed accordantly
trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/CounterStatsAggregator.java
<https://reviews.apache.org/r/16026/#comment57281>
would it better to just adding ", e)" in the end?
nit: formats
- Navis Ryu
On Dec. 5, 2013, 1:01 a.m., Ashutosh Chauhan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16026/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2013, 1:01 a.m.)
>
>
> Review request for hive and Navis Ryu.
>
>
> Bugs: HIVE-5916
> https://issues.apache.org/jira/browse/HIVE-5916
>
>
> Repository: hive
>
>
> Description
> -------
>
> No need to aggregate statistics collected via counter mechanism
>
>
> Diffs
> -----
>
> trunk/itests/qtest/pom.xml 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/StatsTask.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1547966
> trunk/ql/src/java/org/apache/hadoop/hive/ql/stats/CounterStatsAggregator.java 1547966
> trunk/ql/src/test/queries/clientpositive/stats_counter_partitioned.q PRE-CREATION
> trunk/ql/src/test/results/clientpositive/stats12.q.out 1547966
> trunk/ql/src/test/results/clientpositive/stats13.q.out 1547966
> trunk/ql/src/test/results/clientpositive/stats_counter_partitioned.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/16026/diff/
>
>
> Testing
> -------
>
> Added a new test case. Ran all other stats test.
>
>
> Thanks,
>
> Ashutosh Chauhan
>
>