-
Notifications
You must be signed in to change notification settings - Fork 361
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
[CELEBORN-856] Add mapreduce integration test #2073
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2073 +/- ##
==========================================
+ Coverage 46.57% 46.69% +0.13%
==========================================
Files 166 166
Lines 10743 10746 +3
Branches 981 982 +1
==========================================
+ Hits 5002 5017 +15
+ Misses 5417 5406 -11
+ Partials 324 323 -1 ☔ View full report in Codecov by Sentry. |
ping @FMX please take a look at this PR |
👍 Bravo, looks like somebody solved the dependency hell. I'll review this PR. |
project/CelebornBuild.scala
Outdated
@@ -913,6 +913,31 @@ object MRClientProjects { | |||
) | |||
} | |||
|
|||
def mrIt: Project = { | |||
val hadoopVersion = "3.3.6" |
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.
3.2.4?
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.
Here are some dependency conflicts.
- celeborn relies on guava 14, and hadoop-common for 3.2 relies on guava 27, so whichever version of guava is used here will result in
NoSuchMethodError
.
NoSuchMethodError
log
2023-11-10T11:28:08,905 ERROR [main] org.apache.celeborn.mapreduce.v2.app.MRAppMasterWithCeleborn: Error starting MRAppMaster
java.lang.NoSuchMethodError: com.google.common.base.Preconditions.checkArgument(ZLjava/lang/String;Ljava/lang/Object;)V
at org.apache.hadoop.conf.Configuration.set(Configuration.java:1357) ~[hadoop-common-3.2.4.jar:?]
at org.apache.hadoop.conf.Configuration.set(Configuration.java:1338) ~[hadoop-common-3.2.4.jar:?]
at org.apache.celeborn.mapreduce.v2.app.MRAppMasterWithCeleborn.<init>(MRAppMasterWithCeleborn.java:73) ~[classes/:?]
at org.apache.celeborn.mapreduce.v2.app.MRAppMasterWithCeleborn.main(MRAppMasterWithCeleborn.java:149) ~[classes/:?]
But 3.3 removes some guava dependencies and replaces them with a guava shade.
[HADOOP-17098] Reduce Guava dependency in Hadoop source code
[HADOOP-17288] Use shaded guava from thirdparty
- Since MiniYARNCluster is started with YARN with some shaded packages, but MR depends on
hadoop-mapreduce-client-app
which uses hadoop jar without shaded, so here we have to put MR dependent jar at the top of MR task classpath usingmapreduce.job.user. classpath.first=true
andjob.addFileToClassPath
to put the MR dependent jar at the top of the MR task classpath.
NoSuchMethodError
log
[info] Cause: java.lang.NoSuchMethodError: org.apache.hadoop.yarn.webapp.WebApps$Builder.start(Lorg/apache/hadoop/yarn/webapp/WebApp;Lorg/apache/hadoop/shaded/org/eclipse/jetty/webapp/WebAppContext;)Lorg/apache/hadoop/yarn/webapp/WebApp;
[info] at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.startWepApp(ResourceManager.java:1389)
[info] at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.serviceStart(ResourceManager.java:1498)
[info] at org.apache.hadoop.service.AbstractService.start(AbstractService.java:194)
[info] at org.apache.hadoop.yarn.server.MiniYARNCluster.startResourceManager(MiniYARNCluster.java:376)
[info] at org.apache.hadoop.yarn.server.MiniYARNCluster.access$300(MiniYARNCluster.java:129)
[info] at org.apache.hadoop.yarn.server.MiniYARNCluster$ResourceManagerWrapper.serviceStart(MiniYARNCluster.java:500)
NoSuchMethodError
log
[info] Cause: java.lang.NoSuchMethodError: org.apache.hadoop.security.authentication.server.AuthenticationFilter.constructSecretProvider(Ljavax/servlet/ServletContext;Ljava/util/Properties;Z)Lorg/apache/hadoop/security/authentication/util/SignerSecretProvider;
[info] at org.apache.hadoop.http.HttpServer2.constructSecretProvider(HttpServer2.java:799)
[info] at org.apache.hadoop.http.HttpServer2.<init>(HttpServer2.java:685)
[info] at org.apache.hadoop.http.HttpServer2.<init>(HttpServer2.java:129)
[info] at org.apache.hadoop.http.HttpServer2$Builder.build(HttpServer2.java:478)
[info] at org.apache.hadoop.yarn.webapp.WebApps$Builder.build(WebApps.java:372)
[info] at org.apache.hadoop.yarn.webapp.WebApps$Builder.start(WebApps.java:465)
[info] at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.startWepApp(ResourceManager.java:1389)
[info] at org.apache.hadoop.yarn.server.resourcemanager.ResourceManager.serviceStart(ResourceManager.java:1498)
</details>
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.
Maybe we can upgrade Celeborn's guava to 27 to resolve this issue. HDFS 3.2.4 client can access HDFS 2.x clutster while HDFS 3.3.6 can not access HDFS 2.x cluster.
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.
Spark itself is compatible with Guava 27+, but Hive 2.3.9 does not. I suppose we don't need to enable spark-hive module to do test, thus upgrading Guava 27 and shading it in the client module makes sense to me.
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.
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.
After #2090, Celeborn no longer relies on a specific version of guava, and is compatible with guava 14/27/32.
what about bumping guava to the latest version 32.1.3-jre
, but setting guava to version 27.0-jre
when running MapReduce integration tests?
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.
#2090 is merged. How about now? Does this patch needs some modification?
matrix: | ||
java: | ||
- 8 | ||
- 11 |
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.
jdk 17?
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.
Add jdk 17 is reasonable.
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.
guice 4.0 does not support JDK17 - InaccessibleObjectException
guice 4.0 + JDK17
2023-11-22T10:53:55,768 ERROR [Listener at 0.0.0.0/56065] org.apache.celeborn.mapreduce.v2.app.MRAppMasterWithCeleborn: Error starting MRAppMaster
java.lang.ExceptionInInitializerError: null
at com.google.inject.internal.cglib.reflect.$FastClassEmitter.<init>(FastClassEmitter.java:67) ~[guice-4.0.jar:?]
at com.google.inject.internal.cglib.reflect.$FastClass$Generator.generateClass(FastClass.java:72) ~[guice-4.0.jar:?]
at com.google.inject.internal.cglib.core.$DefaultGeneratorStrategy.generate(DefaultGeneratorStrategy.java:25) ~[guice-4.0.jar:?]
at com.google.inject.internal.cglib.core.$AbstractClassGenerator.create(AbstractClassGenerator.java:216) ~[guice-4.0.jar:?]
at com.google.inject.internal.cglib.reflect.$FastClass$Generator.create(FastClass.java:64) ~[guice-4.0.jar:?]
at com.google.inject.internal.BytecodeGen.newFastClass(BytecodeGen.java:204) ~[guice-4.0.jar:?]
at com.google.inject.internal.ProviderMethod$FastClassProviderMethod.<init>(ProviderMethod.java:256) ~[guice-4.0.jar:?]
at com.google.inject.internal.ProviderMethod.create(ProviderMethod.java:71) ~[guice-4.0.jar:?]
at com.google.inject.internal.ProviderMethodsModule.createProviderMethod(ProviderMethodsModule.java:275) ~[guice-4.0.jar:?]
at com.google.inject.internal.ProviderMethodsModule.getProviderMethods(ProviderMethodsModule.java:144) ~[guice-4.0.jar:?]
at com.google.inject.internal.ProviderMethodsModule.configure(ProviderMethodsModule.java:123) ~[guice-4.0.jar:?]
at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:340) ~[guice-4.0.jar:?]
at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:349) ~[guice-4.0.jar:?]
at com.google.inject.AbstractModule.install(AbstractModule.java:122) ~[guice-4.0.jar:?]
at com.google.inject.servlet.ServletModule.configure(ServletModule.java:52) ~[guice-servlet-4.0.jar:?]
at com.google.inject.AbstractModule.configure(AbstractModule.java:62) ~[guice-4.0.jar:?]
at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:340) ~[guice-4.0.jar:?]
at com.google.inject.spi.Elements.getElements(Elements.java:110) ~[guice-4.0.jar:?]
at com.google.inject.internal.InjectorShell$Builder.build(InjectorShell.java:138) ~[guice-4.0.jar:?]
at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:104) ~[guice-4.0.jar:?]
at com.google.inject.Guice.createInjector(Guice.java:96) ~[guice-4.0.jar:?]
at com.google.inject.Guice.createInjector(Guice.java:73) ~[guice-4.0.jar:?]
at com.google.inject.Guice.createInjector(Guice.java:62) ~[guice-4.0.jar:?]
at org.apache.hadoop.yarn.webapp.WebApps$Builder.build(WebApps.java:394) ~[hadoop-yarn-common-3.2.4.jar:?]
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make protected final java.lang.Class java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int,java.security.ProtectionDomain) throws java.lang.ClassFormatError accessible: module java.base does not "opens java.lang" to unnamed module @689349f1
at java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354) ~[?:?]
LGTM overall. I'll merge this PR to the main after minor improvements are applied and UTs are OK. |
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.
LGTM. Thanks. Merged into main.
@cxzl25 Hi, what's your JIRA ID? |
The "dzcxzl" might be your JIRA ID. So I just assign this JIRA to this user. |
Thank you all for your help! |
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?