[SPARK-36835][FOLLOWUP][BUILD][TEST-HADOOP2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom #34100

sunchao · 2021-09-24T16:56:01Z

What changes were proposed in this pull request?

Fix an issue where Maven may stuck in an infinite loop when building Spark, for Hadoop 2.7 profile.

Why are the changes needed?

After re-enabling createDependencyReducedPom for maven-shade-plugin, Spark build stopped working for Hadoop 2.7 profile and will stuck in an infinitely loop, likely due to a Maven shade plugin bug similar to https://issues.apache.org/jira/browse/MSHADE-148. This seems to be caused by the fact that, under hadoop-2.7 profile, variable hadoop-client-runtime.artifact and hadoop-client-api.artifactare both hadoop-client which triggers the issue.

As a workaround, this changes hadoop-client-runtime.artifact to be hadoop-yarn-api when using hadoop-2.7. Since hadoop-yarn-api is a dependency of hadoop-client, this essentially moves the former to the same level as the latter. It should have no effect as both are dependencies of Spark.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

N/A

sunchao · 2021-09-24T16:57:14Z

let me test both Hadoop profiles here

SparkQA · 2021-09-24T17:58:29Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48120/

dongjoon-hyun · 2021-09-24T18:01:33Z

launcher/pom.xml

+      <!--
+        Only declare for Hadoop 3.2 profile. Otherwise maven-shade-plugin may stuck in an infinite
+        loop building the dependency-reduced pom, perhaps due to a maven bug:
+          https://issues.apache.org/jira/browse/MSHADE-148


Thank you for the pointer. According to the discussion on that issue, it's marked as resolved, but not fixed yet?

Right, it's not fixed yet. See the last comment in the JIRA.

dongjoon-hyun · 2021-09-24T18:11:14Z

It looks reasonable to me. I believe we need @gengliangwang 's sign-off with Hadoop 2.7 testing.

gengliangwang · 2021-09-24T18:25:21Z

retest this please

gengliangwang · 2021-09-24T18:26:02Z

Thank you for the fix @sunchao

SparkQA · 2021-09-24T18:41:15Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48120/

JoshRosen

Running

build/mvn -DskipTests -Phadoop-2.7 clean install

didn't work for me: I ran into Enforcer errors related to duplicate dependencies:

[INFO] --- maven-enforcer-plugin:3.0.0-M2:enforce (enforce-no-duplicate-dependencies) @ spark-core_2.12 ---
[WARNING] Rule 0: org.apache.maven.plugins.enforcer.BanDuplicatePomDependencyVersions failed with message:
Found 1 duplicate dependency declaration in this project:
 - dependencies.dependency[org.apache.hadoop:hadoop-client:jar] ( 2 times )

It seems like there's some sort of interaction between Enforcer and Maven Shade, since I would have expected the non-dependency-reduced build to also fail with the same duplicate dependencies issue.

In order to get this to work, I had to make a similar change in the core and kafka-0-10-assembly builds: 11b34c2

I should probably test this again with all optional profiles / modules enabled (or just copy the profiles used when publishing).

SparkQA · 2021-09-24T19:23:56Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48123/

SparkQA · 2021-09-24T19:41:09Z

Test build #143608 has finished for PR 34100 at commit 4456fc1.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-09-24T20:07:01Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48123/

sunchao · 2021-09-24T20:10:49Z

Thanks @JoshRosen ! it's interesting that this error is not reported in the CI jobs. So the enforcer rule is only executed in install phase but not package?

Let me double verify locally and add the changes to all the necessary modules.

SparkQA · 2021-09-24T20:51:02Z

Test build #143611 has finished for PR 34100 at commit 4456fc1.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

sunchao · 2021-09-24T22:31:19Z

updated the PR to use different name for hadoop-client-runtime.artifact, which is probably a simpler approach. Verified locally with:

build/mvn clean install -DskipTests -Phadoop-2.7 -Phive-2.3 -Pmesos -Phive-thriftserver -Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Pkubernetes -Phadoop-cloud -Phive

and the build is successful.

JoshRosen · 2021-09-24T23:10:46Z

pom.xml

        <commons-io.version>2.4</commons-io.version>
        <hadoop-client-api.artifact>hadoop-client</hadoop-client-api.artifact>
-        <hadoop-client-runtime.artifact>hadoop-client</hadoop-client-runtime.artifact>
+        <hadoop-client-runtime.artifact>hadoop-yarn-api</hadoop-client-runtime.artifact>


Ahhh, this is a clever fix:

Instead of the hadoop-2.7 profile resulting in a duplicate direct dependency on hadoop-client, we now just declare an explicit dependency on one of hadoop-client's transitive dependencies (hadoop-yarn-api in this case). Anything which depends on hadoop-client-runtime.artifact must also depend on hadoop-client-api.artifact, so this doesn't end up changing the set of dependencies pulled in.

It looks like we didn't need to do that for hadoop-client-minicluster.artifact because that's only used in the resource-managers/yarn POM and that's already using Maven profiles to control the dependency selection (so the other workaround is fairly non-invasive in that context). In principle, though, I guess we could have changed that to some other transitive dep.

Could you maybe add a one or two line comment above these Hadoop 2.7 lines to explain what's going on? And maybe edit the comment at

spark/pom.xml

Lines 251 to 255 in d73562e



to reflect this change? This fix is clever but a little subtle, so I think a comment calling it out (and maybe mentioning SPARK-36835 might help future readers.

Edit: could you also update the PR description to reflect this final fix?

Thanks for taking a look. Yes I think it's better to apply the same for hadoop-client-minicluster.artifact. Let me try that, and perhaps we won't need the changes in YARN's pom.xml with this.

The side effect for this is seems to be that it affects the distance of these dependencies to the root module and thus may make a difference when maven tries to resolve a dependency with multiple versions (see here for reference). I was using hadoop-common (which carries lots of dependencies) instead of hadoop-yarn-api and it was not able to compile.

Will update PR description and the comment in the above pom.xml.

Actually it may not be so useful to change hadoop-client-minicluster.artifact since it is test scope while the other two are compile scope by default. For some reason it also changes dev/deps/spark-deps-hadoop-2.7-hive-2.3 when I set it to something like hadoop-mapreduce-client-jobclient.

SparkQA · 2021-09-24T23:16:53Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48127/

SparkQA · 2021-09-25T00:01:00Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48127/

SparkQA · 2021-09-25T01:06:51Z

Test build #143615 has finished for PR 34100 at commit d73562e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-09-25T01:58:07Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48130/

SparkQA · 2021-09-25T02:43:12Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48130/

SparkQA · 2021-09-25T03:34:56Z

Test build #143618 has finished for PR 34100 at commit 0c358b3.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gengliangwang · 2021-09-25T07:33:16Z

I tried build/mvn -DskipTests -Phadoop-2.7 clean install and it works now. Shall we merge this and start RC5?

dongjoon-hyun · 2021-09-25T23:34:55Z

Is there any change in your previous opinion, @JoshRosen ?

[SPARK-36835][FOLLOWUP][BUILD][TEST-HADOOP2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom #34100 (comment)

LuciferYang · 2021-09-26T03:47:38Z

branch-3.2 also seems to need this fix

JoshRosen

This looks good to me, so +1 to merging so we can cut a new RC. Thanks again for the fix!

gengliangwang · 2021-09-26T05:39:09Z

Merging to master/3.2. Thanks all!

…doop 2.7 profile after enabling dependency reduced pom ### What changes were proposed in this pull request? Fix an issue where Maven may stuck in an infinite loop when building Spark, for Hadoop 2.7 profile. ### Why are the changes needed? After re-enabling `createDependencyReducedPom` for `maven-shade-plugin`, Spark build stopped working for Hadoop 2.7 profile and will stuck in an infinitely loop, likely due to a Maven shade plugin bug similar to https://issues.apache.org/jira/browse/MSHADE-148. This seems to be caused by the fact that, under `hadoop-2.7` profile, variable `hadoop-client-runtime.artifact` and `hadoop-client-api.artifact`are both `hadoop-client` which triggers the issue. As a workaround, this changes `hadoop-client-runtime.artifact` to be `hadoop-yarn-api` when using `hadoop-2.7`. Since `hadoop-yarn-api` is a dependency of `hadoop-client`, this essentially moves the former to the same level as the latter. It should have no effect as both are dependencies of Spark. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? N/A Closes #34100 from sunchao/SPARK-36835-followup. Authored-by: Chao Sun <[email protected]> Signed-off-by: Gengliang Wang <[email protected]> (cherry picked from commit 937a74e) Signed-off-by: Gengliang Wang <[email protected]>

dongjoon-hyun · 2021-09-26T05:43:20Z

Thank you!

gengliangwang · 2021-09-26T17:05:24Z

@sunchao Unfortunately, the build without Hadoop failed after this one.

$ ./build/mvn clean package -DskipTests -B -Pmesos -Pyarn -Pkubernetes -Psparkr -Pscala-2.12 -Phadoop-provided

[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala:30: object MiniYARNCluster is not a member of package org.apache.hadoop.yarn.server
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala:61: not found: type MiniYARNCluster
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala:104: not found: type MiniYARNCluster
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:37: object resourcemanager is not a member of package org.apache.hadoop.yarn.server
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:38: object resourcemanager is not a member of package org.apache.hadoop.yarn.server
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:39: object resourcemanager is not a member of package org.apache.hadoop.yarn.server
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:40: object resourcemanager is not a member of package org.apache.hadoop.yarn.server
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:250: not found: type RMContext
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:252: not found: type RMApp
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:261: not found: type RMApplicationHistoryWriter
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:263: not found: type SystemMetricsPublisher
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:267: not found: type RMAppManager
[ERROR] [Error] /opt/spark-rm/output/spark-3.2.0-bin-without-hadoop/resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala:272: not found: type ClientRMService

Could you fix it?

sunchao · 2021-09-26T17:34:43Z

@gengliangwang will take a look - is it caused by this PR?

sunchao · 2021-09-26T17:38:05Z

oooh I see, it is because only one active profile is allowed, and thus when hadoop-provided is activated, hadoop-3.2 will not be, and the Hadoop dependencies will not be found.

sunchao · 2021-09-26T17:40:06Z

I think adding -Phadoop-3.2 should make it work but let me think how to fix this.

build/mvn clean package -DskipTests -B -Pmesos -Pyarn -Pkubernetes -Psparkr -Pscala-2.12 -Phadoop-provided -Phadoop-3.2

gengliangwang · 2021-09-26T17:48:39Z

@sunchao Yes it is caused by this one. Again, thanks for looking into this.

sunchao · 2021-09-26T18:00:55Z

There are two ways to fix this:

move spark.yarn.isHadoopProvided to Spark parent pom, so that -Phadoop-3.2 can become the default profile in the YARN module's pom. I don't see any side effect on this - ideally this property can be more general such as spark.isHadoopProvided.
move hadoop-client-runtime.artifact out of the -Phadoop-3.2 profile. It should fix the build issue but someone that's using -Phadoop-provided to test Hadoop 3.2 could still fail.

I'm inclined to option 1) here but let me know if you have any thoughts on this.

gengliangwang · 2021-09-27T03:30:57Z

@sunchao yes let's try option 1

first commit

4456fc1

sunchao changed the title ~~[SPARK-36835][BUILD][hadoop-2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom~~ [SPARK-36835][FOLLOWUP][BUILD][hadoop-2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom Sep 24, 2021

github-actions bot added the BUILD label Sep 24, 2021

sunchao mentioned this pull request Sep 24, 2021

[SPARK-36835][BUILD] Enable createDependencyReducedPom for Maven shaded plugin #34085

Closed

dongjoon-hyun reviewed Sep 24, 2021

View reviewed changes

gengliangwang changed the title ~~[SPARK-36835][FOLLOWUP][BUILD][hadoop-2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom~~ [SPARK-36835][FOLLOWUP][BUILD][TEST-HADOOP2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom Sep 24, 2021

JoshRosen reviewed Sep 24, 2021

View reviewed changes

use different names

d73562e

github-actions bot added the YARN label Sep 24, 2021

JoshRosen reviewed Sep 24, 2021

View reviewed changes

add more comments

0c358b3

sunchao force-pushed the SPARK-36835-followup branch from 73ba941 to 0c358b3 Compare September 25, 2021 00:59

gengliangwang approved these changes Sep 25, 2021

View reviewed changes

dongjoon-hyun approved these changes Sep 25, 2021

View reviewed changes

HyukjinKwon approved these changes Sep 26, 2021

View reviewed changes

JoshRosen approved these changes Sep 26, 2021

View reviewed changes

sunchao mentioned this pull request Sep 26, 2021

[SPARK-33212][BUILD] Upgrade to Hadoop 3.2.2 and move to shaded clients for Hadoop 3.x profile #30701

Closed

gengliangwang closed this in 937a74e Sep 26, 2021

	<!--
	These default to Hadoop 3.x shaded client/minicluster jars, but are switched to hadoop-client
	when the Hadoop profile is hadoop-2.7, because these are only available in 3.x. Note that,
	as result we have to include the same hadoop-client dependency multiple times in hadoop-2.7.
	-->

[SPARK-36835][FOLLOWUP][BUILD][TEST-HADOOP2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom #34100

[SPARK-36835][FOLLOWUP][BUILD][TEST-HADOOP2.7] Fix maven issue for Hadoop 2.7 profile after enabling dependency reduced pom #34100

Uh oh!

Conversation

sunchao commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

sunchao commented Sep 24, 2021

Uh oh!

SparkQA commented Sep 24, 2021

Uh oh!

dongjoon-hyun Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

sunchao Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Sep 24, 2021

Uh oh!

gengliangwang commented Sep 24, 2021

Uh oh!

gengliangwang commented Sep 24, 2021

Uh oh!

SparkQA commented Sep 24, 2021

Uh oh!

JoshRosen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 24, 2021

Uh oh!

SparkQA commented Sep 24, 2021

Uh oh!

SparkQA commented Sep 24, 2021

Uh oh!

sunchao commented Sep 24, 2021

Uh oh!

SparkQA commented Sep 24, 2021

Uh oh!

sunchao commented Sep 24, 2021

Uh oh!

JoshRosen Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunchao Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunchao Sep 25, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 24, 2021

Uh oh!

SparkQA commented Sep 25, 2021

Uh oh!

SparkQA commented Sep 25, 2021

Uh oh!

SparkQA commented Sep 25, 2021

Uh oh!

SparkQA commented Sep 25, 2021

Uh oh!

SparkQA commented Sep 25, 2021

Uh oh!

gengliangwang commented Sep 25, 2021

Uh oh!

dongjoon-hyun commented Sep 25, 2021

Uh oh!

LuciferYang commented Sep 26, 2021

Uh oh!

JoshRosen left a comment

Choose a reason for hiding this comment

Uh oh!

gengliangwang commented Sep 26, 2021

Uh oh!

dongjoon-hyun commented Sep 26, 2021

sunchao commented Sep 24, 2021 •

edited

Loading

JoshRosen left a comment •

edited

Loading

JoshRosen Sep 24, 2021 •

edited

Loading

sunchao Sep 24, 2021 •

edited

Loading