You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Jarek Cecho <ja...@apache.org> on 2013/10/03 23:59:36 UTC

Review Request 14472: PIG-3496 Propagate HBase 0.95 jars to the backend

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

Review request for pig.


Bugs: PIG-3496
    https://issues.apache.org/jira/browse/PIG-3496


Repository: pig-git


Description
-------

I've added the additional required jars into the initialiseHBaseClassLoaderResources() method, so that they get propagated into the backend.


Diffs
-----

  src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 67aa984 

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


Testing
-------

Unit tests for both hbaseversion = 94 | 95 seems to be passing:

ant clean test -Dtestcase=TestHBaseStorage -Dhbaseversion=94 -Dprotobuf-java.version=2.4.0a
ant clean test -Dtestcase=TestHBaseStorage -Dhbaseversion=95 -Dprotobuf-java.version=2.5.0

I've also tried the patch on fully distributed clusters running both major HBase versions and everything seems to be working.


Thanks,

Jarek Cecho


Re: Review Request 14472: PIG-3496 Propagate HBase 0.95 jars to the backend

Posted by Jarek Cecho <ja...@apache.org>.

> On Oct. 4, 2013, 6:31 p.m., Xuefu Zhang wrote:
> >

Thank you for reviewing this patch Xuefu, greatly appreciated! Very good questions!


> On Oct. 4, 2013, 6:31 p.m., Xuefu Zhang wrote:
> > src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java, line 734
> > <https://reviews.apache.org/r/14472/diff/1/?file=361139#file361139line734>
> >
> >     Do we need vendor specific class here?

I know that it seems very weird that "org.apache.hbase" code is depending on "org.cloudera.htrace" package, however this is exposed by HBase itself, so there is not much we can do about it. Please take a look at the HBase pom.xml file where they have this dependency defined:

https://github.com/apache/hbase/blob/trunk/pom.xml#L1352

I've tried the patch on HBase 0.95 cluster without this class and I was getting "ClassNotFound" exceptions, so unfortunately this dependency is necessary.


> On Oct. 4, 2013, 6:31 p.m., Xuefu Zhang wrote:
> > src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java, line 737
> > <https://reviews.apache.org/r/14472/diff/1/?file=361139#file361139line737>
> >
> >     Will old version of HBase continue working?

Yes, I've verified the patch on real HBase clusters running 0.94 and 0.95.


> On Oct. 4, 2013, 6:31 p.m., Xuefu Zhang wrote:
> > src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java, line 735
> > <https://reviews.apache.org/r/14472/diff/1/?file=361139#file361139line735>
> >
> >     The whole thing seems a hack to me. Do we have to do this this way?

I was thinking about having a version check like "if version 0.94, then do that, if version 0.95 then do that", but it seemed unnecessary heavy approach to me as such code would have to be maintained for future possible HBase versions. Instead I've implemented this "try and ignore" approach. The reasoning behind it is that at this code point, we have working frontend  and we are just trying to make sure that all necessary jars are also available on the backend. But since we have working frontend all required jars must be necessary available otherwise frontend itself would fail with ClassNotFound exception. As a result current patch is trying to propagate all known jars that might be needed by HBase and is not explicitly checking if that operation has succeeded.


- Jarek


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


On Oct. 3, 2013, 9:59 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14472/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2013, 9:59 p.m.)
> 
> 
> Review request for pig.
> 
> 
> Bugs: PIG-3496
>     https://issues.apache.org/jira/browse/PIG-3496
> 
> 
> Repository: pig-git
> 
> 
> Description
> -------
> 
> I've added the additional required jars into the initialiseHBaseClassLoaderResources() method, so that they get propagated into the backend.
> 
> 
> Diffs
> -----
> 
>   src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 67aa984 
> 
> Diff: https://reviews.apache.org/r/14472/diff/
> 
> 
> Testing
> -------
> 
> Unit tests for both hbaseversion = 94 | 95 seems to be passing:
> 
> ant clean test -Dtestcase=TestHBaseStorage -Dhbaseversion=94 -Dprotobuf-java.version=2.4.0a
> ant clean test -Dtestcase=TestHBaseStorage -Dhbaseversion=95 -Dprotobuf-java.version=2.5.0
> 
> I've also tried the patch on fully distributed clusters running both major HBase versions and everything seems to be working.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>


Re: Review Request 14472: PIG-3496 Propagate HBase 0.95 jars to the backend

Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14472/#review26685
-----------------------------------------------------------



src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java
<https://reviews.apache.org/r/14472/#comment51974>

    Do we need vendor specific class here?



src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java
<https://reviews.apache.org/r/14472/#comment51975>

    The whole thing seems a hack to me. Do we have to do this this way?



src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java
<https://reviews.apache.org/r/14472/#comment51978>

    Will old version of HBase continue working?


- Xuefu Zhang


On Oct. 3, 2013, 9:59 p.m., Jarek Cecho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14472/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2013, 9:59 p.m.)
> 
> 
> Review request for pig.
> 
> 
> Bugs: PIG-3496
>     https://issues.apache.org/jira/browse/PIG-3496
> 
> 
> Repository: pig-git
> 
> 
> Description
> -------
> 
> I've added the additional required jars into the initialiseHBaseClassLoaderResources() method, so that they get propagated into the backend.
> 
> 
> Diffs
> -----
> 
>   src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 67aa984 
> 
> Diff: https://reviews.apache.org/r/14472/diff/
> 
> 
> Testing
> -------
> 
> Unit tests for both hbaseversion = 94 | 95 seems to be passing:
> 
> ant clean test -Dtestcase=TestHBaseStorage -Dhbaseversion=94 -Dprotobuf-java.version=2.4.0a
> ant clean test -Dtestcase=TestHBaseStorage -Dhbaseversion=95 -Dprotobuf-java.version=2.5.0
> 
> I've also tried the patch on fully distributed clusters running both major HBase versions and everything seems to be working.
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>