STORM-3061: Update version of hbase#2691
Conversation
| <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> |
There was a problem hiding this comment.
Since we are upgrading anyway, do we want to attempt Hbase 2.0 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
@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. |
|
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. |
|
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. |
|
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. |
HeartSaVioR
left a comment
HeartSaVioR
left a comment
There was a problem hiding this comment.
I guess the patch itself gives benefit though we may want to support HBase 2.0 afterwards.
So I'm +1 on the change.
|
@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. |
|
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. |
revans2
force-pushed
the
STORM-3061-hbase-cleanup
branch
from
63506a0 to
4cda7c0
Compare
|
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. |
|
+1, Looks good. |
HeartSaVioR
left a comment
HeartSaVioR
left a comment
There was a problem hiding this comment.
Looks great overall, left minor and nits.
| @@ -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. |
There was a problem hiding this comment.
nit:
skip to setting -> skip setting
instructiuons -> instruction
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
0.94 is the most up to date version of the book, sadly. But it still works.
| <version>${project.version}</version> | ||
| <scope>${provided.scope}</scope> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Did you check with delegation token too? If it doesn't affect delegation token, then removing this would be even better.
There was a problem hiding this comment.
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.
|
@HeartSaVioR I updated the docs from you comments. Thanks for the review. |
HeartSaVioR
left a comment
HeartSaVioR
left a comment
There was a problem hiding this comment.
+1
@revans2
Thanks for addressing comments quickly!
4 participants
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 hbase-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.