You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jesús Camacho Rodríguez <jc...@hortonworks.com> on 2017/02/22 15:47:28 UTC
Review Request 56929: HIVE-12492
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56929/
-----------------------------------------------------------
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-12492
https://issues.apache.org/jira/browse/HIVE-12492
Repository: hive-git
Description
-------
HIVE-12492
Diffs
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3777fa96ba8eac79e07e454ee437fb05583158c5
itests/src/test/resources/testconfiguration.properties 4a69bcc9d4147950dc38955353575ecd01e781d8
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 93e36316770bb5a2d1d60e1317fbddb1eec9c4a2
ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java 61f13749a86473a2dc30eeb5ca86142e6c6599f4
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java 0da7ea434e0d75c1d07044ec5dfe7de673e22768
ql/src/test/queries/clientpositive/join_max_hashtable.q PRE-CREATION
ql/src/test/queries/clientpositive/tez_dynpart_hashjoin_1.q aa331f20096a0a63144bb164aafbccf0edce4625
ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out PRE-CREATION
ql/src/test/results/clientpositive/llap/tez_dynpart_hashjoin_1.q.out 33350a75ed4b9a6fc4ab34110d84f44161d980ec
Diff: https://reviews.apache.org/r/56929/diff/
Testing
-------
Thanks,
Jes�s Camacho Rodr�guez
Re: Review Request 56929: HIVE-12492
Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56929/
-----------------------------------------------------------
(Updated Feb. 23, 2017, 9:13 a.m.)
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-12492
https://issues.apache.org/jira/browse/HIVE-12492
Repository: hive-git
Description
-------
HIVE-12492
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3777fa96ba8eac79e07e454ee437fb05583158c5
itests/src/test/resources/testconfiguration.properties 4a69bcc9d4147950dc38955353575ecd01e781d8
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 93e36316770bb5a2d1d60e1317fbddb1eec9c4a2
ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java 61f13749a86473a2dc30eeb5ca86142e6c6599f4
ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java 0da7ea434e0d75c1d07044ec5dfe7de673e22768
ql/src/test/queries/clientpositive/join_max_hashtable.q PRE-CREATION
ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/56929/diff/
Testing
-------
Thanks,
Jes�s Camacho Rodr�guez
Re: Review Request 56929: HIVE-12492
Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
> On Feb. 23, 2017, 2:13 a.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out, line 18
> > <https://reviews.apache.org/r/56929/diff/1/?file=1642513#file1642513line18>
> >
> > This should have been
> > Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (SIMPLE_EDGE) or
> > Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (CUSTOM_SIMPLE_EDGE)
> > if logic added in patch kicks in.
> > Currently this looks like a regular map join.
Observe in the q file that I play with the values for the properties so join is converted to MapJoin for some queries and not converted for some others.
I did this to verify that effectively the threshold is working as expected.
Since I will revert changes in tez_dynpart_hashjoin_1.q, I have enabled hive.optimize.dynamic.partition.hashjoin=true; for this test. You can observe that everything is working as expected, i.e.,
Reducer 2 <- Map 1 (SIMPLE_EDGE), Map 3 (SIMPLE_EDGE)
becomes
Reducer 2 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 3 (CUSTOM_SIMPLE_EDGE)
> On Feb. 23, 2017, 2:13 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java, line 1416
> > <https://reviews.apache.org/r/56929/diff/1/?file=1642509#file1642509line1416>
> >
> > return null;
No need; observe we are propagating the Exception.
> On Feb. 23, 2017, 2:13 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java, line 1007
> > <https://reviews.apache.org/r/56929/diff/1/?file=1642508#file1642508line1007>
> >
> > LOG.debug (Table small enough for consideration of broadcast edge but we estimate too many distinct keys in its HT, so not converting to MJ)
Message was after the call to the method, but I have moved the message to this line.
- Jes�s
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56929/#review166439
-----------------------------------------------------------
On Feb. 22, 2017, 3:47 p.m., Jes�s Camacho Rodr�guez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56929/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 3:47 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-12492
> https://issues.apache.org/jira/browse/HIVE-12492
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-12492
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3777fa96ba8eac79e07e454ee437fb05583158c5
> itests/src/test/resources/testconfiguration.properties 4a69bcc9d4147950dc38955353575ecd01e781d8
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 93e36316770bb5a2d1d60e1317fbddb1eec9c4a2
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java 61f13749a86473a2dc30eeb5ca86142e6c6599f4
> ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java 0da7ea434e0d75c1d07044ec5dfe7de673e22768
> ql/src/test/queries/clientpositive/join_max_hashtable.q PRE-CREATION
> ql/src/test/queries/clientpositive/tez_dynpart_hashjoin_1.q aa331f20096a0a63144bb164aafbccf0edce4625
> ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out PRE-CREATION
> ql/src/test/results/clientpositive/llap/tez_dynpart_hashjoin_1.q.out 33350a75ed4b9a6fc4ab34110d84f44161d980ec
>
> Diff: https://reviews.apache.org/r/56929/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jes�s Camacho Rodr�guez
>
>
Re: Review Request 56929: HIVE-12492
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56929/#review166439
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 523)
<https://reviews.apache.org/r/56929/#comment238430>
Good to add some javadoc here for args:
buckets : bucket count for BMJ conversion consideration or reduce count for DHJ conversion consideration.
maxSize: size threshold for map join conversion
checkHashTableEntries: threshold for distinct keys in hash table for MJ. Ignored while called from convertToDHJ.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 984)
<https://reviews.apache.org/r/56929/#comment238389>
max < 1 will be more robust.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 996)
<https://reviews.apache.org/r/56929/#comment238390>
LOG.debug()
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 1003)
<https://reviews.apache.org/r/56929/#comment238392>
LOG.debug
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 1006)
<https://reviews.apache.org/r/56929/#comment238446>
LOG.debug (Table small enough for consideration of broadcast edge but we estimate too many distinct keys in its HT, so not converting to MJ)
ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java (line 1416)
<https://reviews.apache.org/r/56929/#comment238431>
return null;
ql/src/test/queries/clientpositive/tez_dynpart_hashjoin_1.q (lines 55 - 105)
<https://reviews.apache.org/r/56929/#comment238445>
I think these changes are redundant. Lets keep these tests as is. I was just trying to give a reference on how plans look like for DHJ.
ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out (line 18)
<https://reviews.apache.org/r/56929/#comment238439>
This should have been
Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (SIMPLE_EDGE) or
Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (CUSTOM_SIMPLE_EDGE)
if logic added in patch kicks in.
Currently this looks like a regular map join.
ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out (line 95)
<https://reviews.apache.org/r/56929/#comment238442>
This should have been
Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (SIMPLE_EDGE) or
Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (CUSTOM_SIMPLE_EDGE)
if logic added in patch kicks in.
Currently this looks like a regular map join.
ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out (line 172)
<https://reviews.apache.org/r/56929/#comment238441>
This should have been
Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (SIMPLE_EDGE) or
Reducer 3 <- Map 1 (CUSTOM_SIMPLE_EDGE), Map 2 (CUSTOM_SIMPLE_EDGE)
if logic added in patch kicks in.
Currently this looks like a regular map join.
- Ashutosh Chauhan
On Feb. 22, 2017, 3:47 p.m., Jes�s Camacho Rodr�guez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56929/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2017, 3:47 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-12492
> https://issues.apache.org/jira/browse/HIVE-12492
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-12492
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3777fa96ba8eac79e07e454ee437fb05583158c5
> itests/src/test/resources/testconfiguration.properties 4a69bcc9d4147950dc38955353575ecd01e781d8
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 93e36316770bb5a2d1d60e1317fbddb1eec9c4a2
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java 61f13749a86473a2dc30eeb5ca86142e6c6599f4
> ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java 0da7ea434e0d75c1d07044ec5dfe7de673e22768
> ql/src/test/queries/clientpositive/join_max_hashtable.q PRE-CREATION
> ql/src/test/queries/clientpositive/tez_dynpart_hashjoin_1.q aa331f20096a0a63144bb164aafbccf0edce4625
> ql/src/test/results/clientpositive/llap/join_max_hashtable.q.out PRE-CREATION
> ql/src/test/results/clientpositive/llap/tez_dynpart_hashjoin_1.q.out 33350a75ed4b9a6fc4ab34110d84f44161d980ec
>
> Diff: https://reviews.apache.org/r/56929/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jes�s Camacho Rodr�guez
>
>