You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2014/11/05 00:56:46 UTC

Review Request 27599: HIVE-8735 : statistics update can fail due to long paths

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27599/
-----------------------------------------------------------

Review request for hive.


Repository: hive-git


Description
-------

see jira


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java b074ca9 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java 5e317ab 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java 70badf2 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java 4625d27 

Diff: https://reviews.apache.org/r/27599/diff/


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 27599: HIVE-8735 : statistics update can fail due to long paths

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Nov. 5, 2014, 9:20 p.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java, line 38
> > <https://reviews.apache.org/r/27599/diff/1/?file=749770#file749770line38>
> >
> >     Why is this limit required? The clients (StatsTask, FSOp) makes sure the stats key prefix is less than maxPrefixLength (defaults to 150). If it exceeds the prefix is MD5'ed to 17bytes. So the stats key prefix should never exceed 255 varchar default.

As the callstack shows it's not sufficient... I will change the handling to be similar, replacing w/the hashcode


> On Nov. 5, 2014, 9:20 p.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java, line 196
> > <https://reviews.apache.org/r/27599/diff/1/?file=749771#file749771line196>
> >
> >     Isn't it clients responsibility to do all these stuff?

I don't think so... the storage limitation is specific to the storage engine. Client doesn't care about that... I think that setting should not even exist


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27599/#review60033
-----------------------------------------------------------


On Nov. 4, 2014, 11:56 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27599/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2014, 11:56 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java b074ca9 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java 5e317ab 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java 70badf2 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java 4625d27 
> 
> Diff: https://reviews.apache.org/r/27599/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 27599: HIVE-8735 : statistics update can fail due to long paths

Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27599/#review60033
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java
<https://reviews.apache.org/r/27599/#comment101370>

    Why is this limit required? The clients (StatsTask, FSOp) makes sure the stats key prefix is less than maxPrefixLength (defaults to 150). If it exceeds the prefix is MD5'ed to 17bytes. So the stats key prefix should never exceed 255 varchar default.



ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java
<https://reviews.apache.org/r/27599/#comment101372>

    Isn't it clients responsibility to do all these stuff?


- Prasanth_J


On Nov. 4, 2014, 11:56 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27599/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2014, 11:56 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java b074ca9 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java 5e317ab 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java 70badf2 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java 4625d27 
> 
> Diff: https://reviews.apache.org/r/27599/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 27599: HIVE-8735 : statistics update can fail due to long paths

Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27599/#review60099
-----------------------------------------------------------

Ship it!


Ship It!

- Prasanth_J


On Nov. 6, 2014, 3:07 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27599/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2014, 3:07 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java b074ca9 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java 5e317ab 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java 70badf2 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java 4625d27 
> 
> Diff: https://reviews.apache.org/r/27599/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 27599: HIVE-8735 : statistics update can fail due to long paths

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27599/
-----------------------------------------------------------

(Updated Nov. 6, 2014, 3:07 a.m.)


Review request for hive.


Repository: hive-git


Description
-------

see jira


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java b074ca9 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java 5e317ab 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java 70badf2 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java 4625d27 

Diff: https://reviews.apache.org/r/27599/diff/


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 27599: HIVE-8735 : statistics update can fail due to long paths

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27599/
-----------------------------------------------------------

(Updated Nov. 5, 2014, 11:01 p.m.)


Review request for hive.


Repository: hive-git


Description
-------

see jira


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java b074ca9 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java 5e317ab 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java 70badf2 
  ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java 4625d27 

Diff: https://reviews.apache.org/r/27599/diff/


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 27599: HIVE-8735 : statistics update can fail due to long paths

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27599/#review59879
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java
<https://reviews.apache.org/r/27599/#comment101185>

    will remove the TODO


- Sergey Shelukhin


On Nov. 4, 2014, 11:56 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27599/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2014, 11:56 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsAggregator.java b074ca9 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsPublisher.java 5e317ab 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsSetupConstants.java 70badf2 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/jdbc/JDBCStatsUtils.java 4625d27 
> 
> Diff: https://reviews.apache.org/r/27599/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>