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
[SPARK-37600][BUILD] Upgrade to Hadoop 3.3.2 #34855
Conversation
Test build #146048 has finished for PR 34855 at commit
|
Kubernetes integration test starting |
Kubernetes integration test starting |
Kubernetes integration test status failure |
Kubernetes integration test status failure |
Test build #146050 has finished for PR 34855 at commit
|
pom.xml
Outdated
@@ -309,6 +309,17 @@ | |||
</extraJavaTestArgs> | |||
</properties> | |||
<repositories> | |||
<repository> |
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.
We'd remove this before merging? after it's released
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.
Yes, will remove this section once the official 3.3.2 release is out.
@@ -120,7 +120,7 @@ | |||
<sbt.project.name>spark</sbt.project.name> | |||
<slf4j.version>1.7.30</slf4j.version> | |||
<log4j.version>1.2.17</log4j.version> | |||
<hadoop.version>3.3.1</hadoop.version> | |||
<hadoop.version>3.3.2</hadoop.version> |
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 update #34830 (comment) together?
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.
Good point. Will do.
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 update #34830 (comment) together?
+1 on. this
c1a6cb7
to
29da6a8
Compare
hmm somehow |
Is there any update, @sunchao ? |
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.
It seems to pass locally. Could you re-trigger the test simply, @sunchao ?
[info] YarnClusterSuite:
[info] - run Spark in yarn-client mode (10 seconds, 131 milliseconds)
[info] - run Spark in yarn-cluster mode (9 seconds, 90 milliseconds)
[info] - run Spark in yarn-client mode with unmanaged am (8 seconds, 78 milliseconds)
[info] - run Spark in yarn-client mode with different configurations, ensuring redaction (10 seconds, 102 milliseconds)
[info] - run Spark in yarn-cluster mode with different configurations, ensuring redaction (10 seconds, 96 milliseconds)
[info] - yarn-cluster should respect conf overrides in SparkHadoopUtil (SPARK-16414, SPARK-23630) (9 seconds, 116 milliseconds)
[info] - SPARK-35672: run Spark in yarn-client mode with additional jar using URI scheme 'local' (10 seconds, 111 milliseconds)
[info] - SPARK-35672: run Spark in yarn-cluster mode with additional jar using URI scheme 'local' (9 seconds, 96 milliseconds)
[info] - SPARK-35672: run Spark in yarn-client mode with additional jar using URI scheme 'local' and gateway-replacement path (8 seconds, 79 milliseconds)
[info] - SPARK-35672: run Spark in yarn-cluster mode with additional jar using URI scheme 'local' and gateway-replacement path (9 seconds, 90 milliseconds)
[info] - SPARK-35672: run Spark in yarn-cluster mode with additional jar using URI scheme 'local' and gateway-replacement path containing an environment variable (9 seconds, 100 milliseconds)
...
orc-core/1.7.3//orc-core-1.7.3.jar | ||
orc-mapreduce/1.7.3//orc-mapreduce-1.7.3.jar | ||
orc-shims/1.7.3//orc-shims-1.7.3.jar | ||
org.jacoco.agent/0.8.5/runtime/org.jacoco.agent-0.8.5-runtime.jar |
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.
jacoco is Java code coverage library, I was surprised that it would become a dependency
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.
oh, I missed this. Do you want to exclude this as an workaround, @sunchao ?
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.
Hmm let me check.
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.
This is a test-only dependency brought in by aliyun-java-sdk-core
in hadoop-cloud-storage
.
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.
Are you sure it's test only? Didn't think those appeared in these drops 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.
I tracked to this commit which introduced it: aliyun/aliyun-openapi-java-sdk@e0d21a3, which looks only used in test?
I manually test with mvn locally, and there will be UT failed:
|
Oh... |
Sorry, this may be my bad. I re-run it twice and succeeded |
Ya, your failed test case is already passed in the original GitHub Action run. Maybe you might hit some flaky test case failure which is still in this module. |
Thanks for helping to verify this @LuciferYang @dongjoon-hyun ! yea it seems a bit flaky. I tried to look into the YARN logs locally but couldn't find anything interesting. Let me try to re-trigger the GitHub workflow. |
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.
@sunchao . All tests except pyspark-pandas-slow
seems to passed. It would be irrelevant.
Could you remove [WIP]
and re-trigger once more, please?
Sure @dongjoon-hyun . Just re-triggered the jobs. |
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, LGTM (Pending CIs). Thank you, @sunchao .
I believe this is almost one. Could you review this once more, @viirya , @srowen , @HyukjinKwon , @AngersZhuuuu , @LuciferYang ? |
To @sunchao . It seems that it's not re-triggered yet. You may want to add an empty commit. |
Re-triggered via empty commit. I did it manually by clicking the "Re-run all jobs" button which wasn't reflected here somehow. |
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 +1
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.
Licenses look OK
@srowen , do you have any other concerns? Or, the last issue (LICENSE) is resolved and we are good to go? |
@@ -69,7 +69,7 @@ private[hive] object IsolatedClientLoader extends Logging { | |||
// If the error message contains hadoop, it is probably because the hadoop | |||
// version cannot be resolved. | |||
val fallbackVersion = if (VersionUtils.isHadoop3) { | |||
"3.3.1" | |||
"3.3.2" | |||
} else { | |||
"2.7.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.
By the way, can we read the hadoop version of the project configuration here?
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.
That sounds like independent improvement idea. Could you file a JIRA for that?
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.
That sounds like independent improvement idea. Could you file a JIRA for that?
Yea, will try to do this.
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.
I'm not sure this is easy since in this case the Hadoop version specified via hadoop.version
in pom.xml
is customized and is not 3.3.2
, which is why it can't be fetched from Maven.
Thank you, @sunchao , @viirya , @srowen , @HyukjinKwon , @LuciferYang , @AngersZhuuuu . Also, cc @MaxGekk since he is the release manager for Apache Spark 3.3. |
What changes were proposed in this pull request?
This PR aims to upgrade to Hadoop 3.3.2. In addition, it also removes the LZ4 wrapper classes added in SPARK-36669, therefore fixing SPARK-36679.
Why are the changes needed?
Hadoop 3.3.2 has many bug fixes and we also can remove our internal hacked Hadoop codecs.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass the CIs.