You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/11/18 12:12:19 UTC
[GitHub] [iceberg] marton-bod opened a new pull request #1781: Package thrift into the hive-runtime jar
marton-bod opened a new pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781
Include thrift into the hive-runtime jar to enable Tez AM - HMS commucation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
pvary commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r526041426
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303:0.9.3")
Review comment:
Shouldn't we use the versions.props to define the required version?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
marton-bod commented on pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#issuecomment-739967840
Closing this PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod closed pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
marton-bod closed pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shardulm94 commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
shardulm94 commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r526584964
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Shouldn't this already be provided by Hive at runtime? Why do we have to include this?
If we need to include this, is it okay to not shade this? We usually shade non Iceberg libraries in the runtime jars so that there aren't multiple jars with same class names on the classpath, one from Iceberg and another from the execution engine, causing conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] massdosage commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
massdosage commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r537609574
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
I just want to confirm whether this is only a problem with Hive 3? I've set up Iceberg tables and queried them from Hive 2 using HadoopTables, HadoopCatalog and HiveCatalog using MapReduce and Tez and didn't run into this issue. If we could figure out the exact combination of Hive/Tez/Iceberg that is causing this it might dictate which is the best solution?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r537620827
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Yes, the problem came up for me on a Hive3 cluster and only for HiveCatalog. I haven't had a chance to test this on Hive2, but it's good to know it's not a problem there. Did you run your Tez tests on a cluster where Tez spawns different processes for the tasks? Not sure yet what the difference could be but it's worth investigating.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] massdosage commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
massdosage commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r530472600
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
I've never run into a problem with Iceberg on Hive related to this as Hive provides `libfb303-0.9.3.jar` in its lib folder (for Hive 2.3.7). What steps reproduce the problem? Is it specific to Hive 3?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r532489372
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Apologies, I've been dragged into other efforts for the last week or so, hence the lack of time to investigate further. I'll respond as soon as I get back to Iceberg-ing finally :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r527791291
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Thanks @shardulm94 , very good points.
Tez is responsible for its own classpath, Hive does not provide dependencies to the Tez AM.
Right now pre-iceberg, the Tez AM does not communicate with HMS, therefore has not needed the thrift library on its classpath before, only with Iceberg tables now. Hence, the inclusion in the runtime jar.
I'm not sure it will work and find the classes properly if we use shading - this will need to be tested. In theory I think it could be okay not to shade in this specific case, since we are putting the same thrift version on the classpath as the one already used by Hive. Using Hive with a thrift version other than 0.9.3 would likely break Hive so having different thrift versions on the classpath is not a hugely realistic cause for breakage I believe.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r535277851
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Thanks for your patience again. Here are my answers:
> Shouldn't this already be provided by Hive at runtime? Should Hive be the one putting this on the AM?
Correct. Hive localizes the hive exec jar onto the classpath of the Tez AM, via `TezSessionState.createJarLocalResource`. More precisely, it checks which jar the class `org.apache.hadoop.hive.ql.exec.tez.DagUtils` comes from and localizes that specific jar for the AM
-- and this should be the `hive-exec.jar`.
> Also another interesting consideration is that Iceberg does not use Thrift directly, it uses HMSClient. Now hive-runtime does not include HMSClient code either. So how is Tez AM able to get the HMSClient code but not Thrift?
Correct, it's the `HiveMetaStoreClient` which we use directly in Iceberg. This class (along with other metastore classes) is already part of the `hive-exec.jar`, along with most of the Thrift classes as well so Tez has no problem finding them. The only thing that's missing is the `libfb303` (Facebook) thrift classes - these are not contained in the `hive-exec.jar`. This absence has not caused any issues before because Tez has not needed to communicate with the HMS, but now it does when we use Iceberg tables.
> What is Iceberg doing on the AM that needs to talk with HMS?
Mostly `IcebergInputFormat.getSplits` loading the table from HMS (and previously `HiveIcebergSerDe` loading the table schema from the HMS - but this latter should be taken care of by https://github.com/apache/iceberg/pull/1794)
> I've never run into a problem with Iceberg on Hive related to this as Hive provides libfb303-0.9.3.jar in its lib folder (for Hive 2.3.7). What steps reproduce the problem? Is it specific to Hive 3?
This problem should be specific to Tez, because the AM does not have access to the lib folder used by Hive. Not exactly sure how the classpath is handled when MR launches separate processes.
> If we need to include this, is it okay to not shade this?
I've tried shading it by including `relocate 'com.facebook.fb303', 'org.apache.iceberg.shaded.com.facebook.fb303'` but then Hive unfortunately failed to find the class when it was shaded.
So to summarize:
- In the current state, Tez does not work out-of-the-box using only the hive-iceberg-runtime jar. The user also needs to put the `libfb303` jar on Tez's classpath one way or another. This should be documented. I will create a PR to add this to the Hive docs.
- While the solution in this PR does solve this problem, the `libfb303` classes don't really belong in the runtime jar, especially in an unshaded version (although probably not too risky given there'd be no version conflict since Hive2 and Hive3 both use 0.9.3), so I'll abandon this change
- A probably better solution would be to create a HIVE change and include the `libfb303` classes in the `hive-exec.jar` (which already contains the other thrift classes). Then we can include that change in an upcoming upstream release and upgrade the version in Iceberg once it's out.
What are your thoughts?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shardulm94 commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
shardulm94 commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r526584964
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Shouldn't this already be provided by Hive at runtime? Should Hive be the one putting this on the AM?
If we need to include this, is it okay to not shade this? We usually shade non Iceberg libraries in the runtime jars so that there aren't multiple jars with same class names on the classpath, one from Iceberg and another from the execution engine, causing conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] massdosage commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
massdosage commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r537648394
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
I just did some simple SELECT statements with an "ORDER BY" in order to trigger the job to be run on the cluster. Is there something else I could do to try hit the situation you're talking about?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r528020070
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Our Hive Jar works with multiple Hive versions, so I'm concerned about this not being provided.
What is Iceberg doing on the AM that needs to talk with HMS?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] marton-bod commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
marton-bod commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r526042350
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303:0.9.3")
Review comment:
Could do. It's not currently defined there, and both `mr` (hive2) and `hive3` use the 0.9.3 version. But to enforce consistency, I'll move it to version.props.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] pvary commented on pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
pvary commented on pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#issuecomment-729660164
+1 (non-binding)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] shardulm94 commented on a change in pull request #1781: Package thrift into the hive-runtime jar
Posted by GitBox <gi...@apache.org>.
shardulm94 commented on a change in pull request #1781:
URL: https://github.com/apache/iceberg/pull/1781#discussion_r528245175
##########
File path: build.gradle
##########
@@ -607,6 +607,8 @@ project(':iceberg-hive-runtime') {
if (jdkVersion == '8') {
compile project(':iceberg-hive3')
}
+ // including thrift so that the Tez AM can communicate with the HMS when using Hive catalog
+ compile("org.apache.thrift:libfb303")
Review comment:
Also another interesting consideration is that Iceberg does not use Thrift directly, it uses HMSClient. Now hive-runtime does not include HMSClient code either. So how is Tez AM able to get the HMSClient code but not Thrift?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org