HADOOP-18303. Remove shading exclusion of javax.ws.rs-api from hadoop-client-runtime #4461
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
virajjasani
changed the title
steveloughran
left a comment
steveloughran
left a comment
There was a problem hiding this comment.
+1 pending my comments.
I'm confident that the javadoc failure is unrelated, are we all happy that the hdfs one is too?
pom.xml
Outdated
There was a problem hiding this comment.
cut this change; i want the most minimal backport possible
There was a problem hiding this comment.
Sure thing, this was done just to trigger the full test run (and hence we see hdfs javadoc failure because all modules were involved in the build runs)
|
hdfs one is my fault :( |
virajjasani
force-pushed
the
javax.ws.rs-api-exclude
branch
from
1e2a70b to
3f6c6e0
Compare
There was a problem hiding this comment.
Now we have both in shaded jar
[INFO] Including javax.ws.rs:javax.ws.rs-api:jar:2.1.1 in the shaded jar.
[INFO] Including javax.ws.rs:jsr311-api:jar:1.1.1 in the shaded jar.
Both have couple of exactly same classes like
for UriBuilder
ayushsaxena@ayushsaxena-MBP16 hadoop % jar tf ~/.m2/repository/javax/ws/rs/javax.ws.rs-api/2.1.1/javax.ws.rs-api-2.1.1.jar | grep UriBuilder.class
javax/ws/rs/core/UriBuilder.class
ayushsaxena@ayushsaxena-MBP16 hadoop % jar tf ~/.m2/repository/javax/ws/rs/jsr311-api/1.1.1/jsr311-api-1.1.1.jar | grep UriBuilder.class
javax/ws/rs/core/UriBuilder.class
But in the shaded jar there is only one UriBuilder.class
ayushsaxena@ayushsaxena-MBP16 target % jar tf hadoop-client-runtime-3.4.0-SNAPSHOT.jar | grep UriBuilder.class
org/apache/hadoop/shaded/javax/ws/rs/core/UriBuilder.class
It is overwriting one on top of other, may be the one which comes last stays....
|
i wouldn't trust the ordering of the merge. could one jar be excluded explicitly. I'm getting really confused now. Before this PR, conflicting imports were both being shaded. Now they are being merged in unshaded and the issue is which ones not to shade? |
I am also confused.... No, one was shaded one wasn't, the one not shaded is getting added here because kirby & spark are crying due to missing classes. The question is yes which one to shade and which one not to, In Ideal situation only one of these jars should be there because they provide similar classes and to be used by different version. In our case Jackson use the new Jar, and Jersey uses the old Jar, so we need both Jar in hadoop. If we upgrade Jersey also, both Jackson & Jersey will need the new jar and we can remove one jar and this conflict. Let us assume we did something with the shading, but Projects like Tez, when they add Hadoop as dependency they get both Jars and both have similar classes, so that creates a mess, something similar error message like here: Regarding Solution: I don't have a perfect solution, and definitely need pointers, Not sure what we can do with shading, if we figure out may be spark/kriby may be happy. But if we exclude the old jar who will be unhappy god knows.... Shading won't solve Hive/Tez issue.... Jersey upgrade: Not sure we can do for a maintenance release, people have objections for protobuf as well, and post that also I guess downstream also have to upgrade Jersey, that won't be easy either. That is all what I know or can think of, Not sure what all is Right/Wrong. Hope I didn't contribute more to the confusion, if I did Please read the first line. :-) |
so why aren't they explicitly asking for the jars? or are some bits of our code are still asking for things by their unshaded classnames? |
|
@pan3793 might be good to answer the kyuubi & spark issue is flagged by him here: @pan3793 can you please share some more details on that... |
|
In this case, the problematic class is
Hmm why is that? does Hive/Tez use shaded Hadoop client? the shading also comes with relocation so that it shouldn't interfere with the other dependencies. |
|
@sunchao It doesn't, that is what I was saying, even if we sort the shading issue, the Tez issue is gonna stay because it doesn't use the shaded Jar and gets both these jars in classpath and things mess up. |
|
Looks like upgrading jersey to get rid of |
ok, but then no aspects of jersey 2 must be unshaded or every project which still uses 1.19 is going to break. we don't want to do that |
I see. This looks like a separate issue from this PR, is that correct? is this because of https://issues.apache.org/jira/browse/HADOOP-18033? The issue seems tricky though. I wonder if we can shade jersey 2 in |
|
Both shading and the issue that I am talking about are due to HADOOP-18033 |
|
In that case should we consider reverting it for now until we are ready to upgrade both jersey and jackson together at some point? |
|
Yeps, if people let us do that, that jira is already in a released version |
|
While HADOOP-18033 is released in 3.3.2, HADOOP-18178 has also made it's way to 3.3.3. |
|
Even if HADOOP-18033 had not excluded javax.ws.rs-api from shading, as mentioned here, we would have anyways caused subtle problems by having only one version of common class in the final client jar (as opposed to having different classes for both javax.ws.rs-api and jsr311-api). |
|
I'm still facing the issue I was using Hadoop 3.1.1 . Now installed Hadoop 3.3.3. Still getting the problem. |
|
Try with 3.3.4, we fixed it there If I remember correctly |
7 participants
Description of PR
Discussion on HADOOP-18033