-
Notifications
You must be signed in to change notification settings - Fork 2.5k
[HUDI-5832] add relocated prefix for hbase classes in hbase-site.xml #8029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for contribution, can we elaborate a little the details? |
|
@danny0405 Thanks for review this. I give more context here. I meet a problem about ClassNotFound exception During open Hbase connection and write data to Hbase, the application will load the So I'm thinking all configured classes properties should start with relocated prefix @yihua could you please help to double check if it's reasonable? |
|
Reasonable, looks good from my side. |
|
they @stayrascal : thanks for the contribution. |
danny0405
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, should good to go
|
@yihua : can you review this. |
| <name>hbase.master.logcleaner.plugins</name> | ||
| <value> | ||
| org.apache.hadoop.hbase.master.cleaner.TimeToLiveLogCleaner,org.apache.hadoop.hbase.master.cleaner.TimeToLiveProcedureWALCleaner,org.apache.hadoop.hbase.master.cleaner.TimeToLiveMasterLocalStoreWALCleaner | ||
| org.apache.hudi.org.apache.hadoop.hbase.master.cleaner.TimeToLiveLogCleaner,org.apache.hudi.org.apache.hadoop.hbase.master.cleaner.TimeToLiveProcedureWALCleaner,org.apache.hudi.org.apache.hadoop.hbase.master.cleaner.TimeToLiveMasterLocalStoreWALCleaner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just exclude the hbase-site.xml from our bundle jars? This hbase-site.xml can cause conflict if other jars in the classpath load the hbase-site.xml file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately we cannot exclude the hbase-site.xml, which is added explicitly for the metadata table to work properly with HFile (see the description of #5004).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read the description in #5004, but still confused why the wrong class name(unshaded) still works here, does that mean the classpath must contain these classes to make the hudi jar run correctly?
Another question is do we need all the config items?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading and writing HFile do not require some of the HBase configs, including the ones fixed by this PR, so solely for reading and writing the metadata table, there should not be any issue. But if other HBase functionality is used elsewhere and the Hudi jar is loaded first by the class loader, the unshaded class names can cause problems.
We need all the configs as they are required for HBase 2.4.9.
Makes sense. LGTM. |
|
@stayrascal could you check the CI failure? |
The shaded pattern can solve this problem, while for |
This is a good question, unfortunately we have no good way to solve it unless we publish our own shaded Hbase jars, i.e all the hudi modules depends on the separately published shaded Hbase jars. |
Ultimately we'll have our own HFile reader and writer implementation and remove HBase dependency for metadata table. |
@yihua Thanks a lot for review this PR. may I check which CI failed? it seems that all CI passed? |
|
There is a failue for timeout: https://dev.azure.com/apache-hudi-ci-org/apache-hudi-ci/_build/results?buildId=15364&view=logs&j=7601efb9-4019-552e-11ba-eb31b66593b2&t=9688f101-287d-53f4-2a80-87202516f5d0&l=6717, should not be cause by the patch. |
|
@yihua @danny0405 : Is this good to be merged ? |
Yes, should be good to land. |
|
@stayrascal : Since this is a old PR, can you please rebase this PR so that we can land it. |
14d1cf6 to
77bc172
Compare
Thanks @nsivabalan a lot for reminder this, just rebase the latest changes from master branch, and update this PR. |
|
The test `` is failing continously after merging the PR: Caused by: java.lang.RuntimeException: java.lang.ClassNotFoundException: Class org.apache.hudi.org.apache.hadoop.hbase.http.lib.StaticUserWebFilter not found
at org.apache.hadoop.conf.Configuration.getClasses(Configuration.java:2404)
at org.apache.hadoop.hbase.http.HttpServer.getFilterInitializers(HttpServer.java:705)
at org.apache.hadoop.hbase.http.HttpServer.initializeWebServer(HttpServer.java:639)
at org.apache.hadoop.hbase.http.HttpServer.<init>(HttpServer.java:588)
at org.apache.hadoop.hbase.http.HttpServer.<init>(HttpServer.java:107)
at org.apache.hadoop.hbase.http.HttpServer$Builder.build(HttpServer.java:411)
at org.apache.hadoop.hbase.http.InfoServer.<init>(InfoServer.java:96)
at org.apache.hadoop.hbase.regionserver.HRegionServer.putUpWebUI(HRegionServer.java:2201)
at org.apache.hadoop.hbase.regionserver.HRegionServer.<init>(HRegionServer.java:688)
at org.apache.hadoop.hbase.master.HMaster.<init>(HMaster.java:417)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at org.apache.hadoop.hbase.util.JVMClusterUtil.createMasterThread(JVMClusterUtil.java:132)
... 65 more
Caused by: java.lang.ClassNotFoundException: Class org.apache.hudi.org.apache.hadoop.hbase.http.lib.StaticUserWebFilter not found
at org.apache.hadoop.conf.Configuration.getClassByName(Configuration.java:2329)
at org.apache.hadoop.conf.Configuration.getClasses(Configuration.java:2400)
... 79 moreThe shaded class can not be found in the classpath. |
…ite.xml (apache#8029)" This reverts commit a6f85b7.
…ite.xml (apache#8029)" This reverts commit a6f85b7.
|
@stayrascal : We are reverting this change. Can you please relook at failing tests and reopen the PR after fixing. |
sorry for reply late, will check it. |
This file (hbase-site.xml) specified 'zookeeper.znode.parent' to '/hbase', some hive query from external hbase table will not available when use another 'zookeeper.znode.parent' config if we not open the config 'metadata.enabled', can we delete this xml file from hudi-hadoop-mr-bundle.jar ? |
Yes, |
Change Logs
Add relocated prefix(org.apache.hudi) for hbase classes in hbase-site.xml, because we have relocated hbase classes in the bundle jar.
Note:
Maybe it's better to put hbase-site.xml in bundle module instead of
hudi-commonmodule. But currently it seems that every bundle jar will relocated hbase classes, so it's fine to modify the hbase-site.xml inhudi-commonmodule directlyImpact
No
Risk level (write none, low medium or high below)
low
Documentation Update
No
Contributor's checklist