You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by revans2 <gi...@git.apache.org> on 2018/05/23 22:02:14 UTC

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

GitHub user revans2 opened a pull request:

    https://github.com/apache/storm/pull/2691

    STORM-3061: Update version of hbase

    This updates the version of hbase used and cleans up some of the dependencies.
    
    The biggest change besides updating the version is that we remove storm-server as a dependency, because it was only used for access to a copy of StringUtils.
    
    This ends up impacting a lot of packages that were pulling in storm-hbase, either directly or indirectly.
    
    storm-autocreds
    strom-hdfs (because it depends on storm-autocreds)
    flux-core (not really sure why the core of flux needs hbase but it is a dependency)
    flux-examples 
    storm-sql-hdfs (because of storm-autocreds)
    storm-hdfs-blobstore (auotcreds again)
    storm-hive (autocreds yet again)
    storm-starter
    storm-hdfs-examples (autocreds)
    storm-hbase-examples
    storm-hive-examples (autocreds)
    storm-perf (autocreds)
    
    I have not run any of the manual tests for this yet.  But I plan on doing some soon.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/revans2/incubator-storm STORM-3061-hbase-cleanup

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/2691.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2691
    
----
commit 63506a0a3b66f7dc2100480282d99fda8537d2f4
Author: Robert (Bobby) Evans <ev...@...>
Date:   2018-05-23T21:53:16Z

    STORM-3061: Update version of hbase

----


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    I am planning right now to get STORM-2882 in first, and then I will come back and do as much manual testing as possible for the different components, and update thing accordingly.


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    @HeartSaVioR I updated the docs from you comments.  Thanks for the review.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r196334549
  
    --- Diff: external/storm-hbase/pom.xml ---
    @@ -42,31 +42,15 @@
                 <version>${project.version}</version>
                 <scope>${provided.scope}</scope>
             </dependency>
    -        <dependency>
    --- End diff --
    
    Did you check with delegation token too? If it doesn't affect delegation token, then removing this would be even better.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r190432849
  
    --- Diff: pom.xml ---
    @@ -294,7 +294,7 @@
             <hive.version>0.14.0</hive.version>
             <hadoop.version>2.6.1</hadoop.version>
             <hdfs.version>${hadoop.version}</hdfs.version>
    -        <hbase.version>1.1.12</hbase.version>
    +        <hbase.version>1.4.4</hbase.version>
    --- End diff --
    
    In 2.0 TokenUtil is a part of hbase-server.  I will spend some time to see if there is something I can do to work around this but I really don't want to require the server code be shipped with all of the clients.  Perhaps I can refactor autocreds so the nimbus portions are split off into a separate package.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r190428653
  
    --- Diff: pom.xml ---
    @@ -294,7 +294,7 @@
             <hive.version>0.14.0</hive.version>
             <hadoop.version>2.6.1</hadoop.version>
             <hdfs.version>${hadoop.version}</hdfs.version>
    -        <hbase.version>1.1.12</hbase.version>
    +        <hbase.version>1.4.4</hbase.version>
    --- End diff --
    
    I am happy to give it a try.  I was a bit cautious with it being a major version change, and that there was no 2.0.1 yet.  But it does fit better with going to hadoop 3.1 coming in another pull request.  Probably after this one is merged in.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r196331644
  
    --- Diff: examples/storm-hbase-examples/README.md ---
    @@ -0,0 +1,62 @@
    +# Storm HBase Integration Example 
    +
    +This is a very simple set of topologies that show how to use the storm-hbase package for accessign HBase from storm.
    --- End diff --
    
    nit: `accessing`


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    Actually I was wrong. storm-hdfs, storm-hbase, storm-hive seem to depend on storm-autocreds. I guess we could pull out the required classes into some common package as part of the follow up JIRA.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r196331757
  
    --- Diff: examples/storm-hbase-examples/README.md ---
    @@ -0,0 +1,62 @@
    +# Storm HBase Integration Example 
    +
    +This is a very simple set of topologies that show how to use the storm-hbase package for accessign HBase from storm.
    +
    +## HBase Setup
    +
    +First you need an instance of HBase that is setup and running.  If you have one already you can skip to setting up the table, if not download a copy from http://archive.apache.org/dist/hbase/1.4.4/ and untar the result into a directory you want to run it in. Then follow the instructiuons from https://hbase.apache.org/0.94/book/quickstart.html to setup a standalone HBase instance.  Be aware that when you run `start-hbase.sh` an instance of zookeeper will also be started.  If you are testing using a single node storm cluster you can skip running zookeeper yourself as the hbase zookeeper instance will work.
    --- End diff --
    
    nit: 
    skip to setting -> skip setting
    instructiuons -> instruction


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    @arunmahadevan I am happy to try and split up autocreds to make that happen, but it is a much larger job than what is currently for this.  If you are fine with waiting I would rather file a follow on JIRA to upgrade to 2.0.0 and split up autocreds instead of blocking this.


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    +1, Looks good.


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    @HeartSaVioR @arunmahadevan 
    
    I rebased my changes on master, I also ran the example topologies. I had to make a minor change around the HBase config to make the trident example work.  I also added in a README to the examples that explains how to run them and verify that they are working.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r196544696
  
    --- Diff: external/storm-hbase/pom.xml ---
    @@ -42,31 +42,15 @@
                 <version>${project.version}</version>
                 <scope>${provided.scope}</scope>
             </dependency>
    -        <dependency>
    --- End diff --
    
    I didn't test delegation tokens, but the code to download it didn't change and is a part of the client.  It moved to the server in 2.x, which is why I didn't go to 2.x right now.  To support 2.x we will want to split up the AutoHbase credentials so we don't need to have the server on the classpath just for that API.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r196544065
  
    --- Diff: examples/storm-hbase-examples/README.md ---
    @@ -0,0 +1,62 @@
    +# Storm HBase Integration Example 
    +
    +This is a very simple set of topologies that show how to use the storm-hbase package for accessign HBase from storm.
    +
    +## HBase Setup
    +
    +First you need an instance of HBase that is setup and running.  If you have one already you can skip to setting up the table, if not download a copy from http://archive.apache.org/dist/hbase/1.4.4/ and untar the result into a directory you want to run it in. Then follow the instructiuons from https://hbase.apache.org/0.94/book/quickstart.html to setup a standalone HBase instance.  Be aware that when you run `start-hbase.sh` an instance of zookeeper will also be started.  If you are testing using a single node storm cluster you can skip running zookeeper yourself as the hbase zookeeper instance will work.
    --- End diff --
    
    0.94 is the most up to date version of the book, sadly.  But it still works.


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    @revans2 Do you still plan to do manual tests on this patch? Sadly I couldn't do it myself for now, so would like to rely on your test result.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r196333499
  
    --- Diff: examples/storm-hbase-examples/README.md ---
    @@ -0,0 +1,62 @@
    +# Storm HBase Integration Example 
    +
    +This is a very simple set of topologies that show how to use the storm-hbase package for accessign HBase from storm.
    +
    +## HBase Setup
    +
    +First you need an instance of HBase that is setup and running.  If you have one already you can skip to setting up the table, if not download a copy from http://archive.apache.org/dist/hbase/1.4.4/ and untar the result into a directory you want to run it in. Then follow the instructiuons from https://hbase.apache.org/0.94/book/quickstart.html to setup a standalone HBase instance.  Be aware that when you run `start-hbase.sh` an instance of zookeeper will also be started.  If you are testing using a single node storm cluster you can skip running zookeeper yourself as the hbase zookeeper instance will work.
    --- End diff --
    
    Looks like reference guide for HBase 1.2.6 doc is a single page which is taking super huge latency, but is quickstart page for 0.94 compatible with 1.4.4? There's huge gap between two versions so just would like to check.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/storm/pull/2691


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    Yes that was the plan.  there is a lot that depends on storm-autocreds and I would like to understand it all better before I try to clean it up.


---

[GitHub] storm pull request #2691: STORM-3061: Update version of hbase

Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2691#discussion_r190424484
  
    --- Diff: pom.xml ---
    @@ -294,7 +294,7 @@
             <hive.version>0.14.0</hive.version>
             <hadoop.version>2.6.1</hadoop.version>
             <hdfs.version>${hadoop.version}</hdfs.version>
    -        <hbase.version>1.1.12</hbase.version>
    +        <hbase.version>1.4.4</hbase.version>
    --- End diff --
    
    Since we are upgrading anyway, do we want to attempt Hbase 2.0 ?


---

[GitHub] storm issue #2691: STORM-3061: Update version of hbase

Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:

    https://github.com/apache/storm/pull/2691
  
    Sure we can revisit this in a follow up JIRA.
    
    We may not have to split the autocreds since none of the other components depends on it. The hbase-server dependency if included is just going to end up under external/storm-autocreds and not going to be included in the class path by default. We could also check with the Hbase team to pull out the TokenUtils into hbase-client package.


---