SPARK-1209 [CORE] SparkHadoop{MapRed,MapReduce}Util should not use package org.apache.hadoop #2814

srowen · 2014-10-15T15:36:36Z

(This is just a look at what completely moving the classes would look like. I know Patrick flagged that as maybe not OK, although, it's private?)

SparkQA · 2014-10-15T15:39:49Z

QA tests have started for PR 2814 at commit ec1b84a.

This patch merges cleanly.

AmplabJenkins · 2014-10-15T16:49:30Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21767/
Test FAILed.

shaneknapp · 2014-10-15T17:04:43Z

jenkins, test this please.

srowen · 2014-10-15T17:05:17Z

Jenkins, test this please.

SparkQA · 2014-10-15T17:18:37Z

QA tests have finished for PR 2814 at commit ec1b84a.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-10-15T17:18:40Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21765/
Test FAILed.

AmplabJenkins · 2014-10-15T17:52:19Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21768/
Test FAILed.

srowen · 2014-10-16T14:04:04Z

I addressed the MIMA warnings, two of which were just caused by the class being renamed.

I'm less sure about the warning around PairRDDFunctions since SparkHadoopMapReduceUtil was included as a superclass (trait). Unfortunately. Right? it really isn't a case for inheritance.
So, I am not 100% clear what the implications of a rename are for binary compatibility.

SparkQA · 2014-10-16T14:09:44Z

QA tests have started for PR 2814 at commit 821d12a.

This patch merges cleanly.

SparkQA · 2014-10-16T15:39:07Z

QA tests have finished for PR 2814 at commit 821d12a.

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

AmplabJenkins · 2014-10-16T15:39:11Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21806/
Test PASSed.

SparkQA · 2014-10-19T22:54:41Z

QA tests have started for PR 2814 at commit 11e7d5d.

This patch merges cleanly.

SparkQA · 2014-10-20T00:24:38Z

QA tests have finished for PR 2814 at commit 11e7d5d.

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

AmplabJenkins · 2014-10-20T00:24:42Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21895/
Test PASSed.

SparkQA · 2014-10-26T18:39:47Z

Test build #22248 has started for PR 2814 at commit 514cb38.

This patch merges cleanly.

SparkQA · 2014-10-26T19:53:25Z

Test build #22248 has finished for PR 2814 at commit 514cb38.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-10-26T19:53:29Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22248/
Test FAILed.

andrewor14 · 2014-10-28T19:53:26Z

retest this please

SparkQA · 2014-10-28T19:59:51Z

Test build #22373 has started for PR 2814 at commit 514cb38.

This patch merges cleanly.

SparkQA · 2014-10-28T21:59:51Z

Test build #22373 timed out for PR 2814 at commit 514cb38 after a configured wait of 120m.

AmplabJenkins · 2014-10-28T21:59:54Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22373/
Test FAILed.

andrewor14 · 2014-10-28T23:00:14Z

retest this please

SparkQA · 2014-10-28T23:02:35Z

Test build #22388 has started for PR 2814 at commit 514cb38.

This patch merges cleanly.

SparkQA · 2014-10-29T00:27:56Z

Test build #22388 has finished for PR 2814 at commit 514cb38.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-10-29T00:27:59Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22388/
Test FAILed.

andrewor14 · 2014-10-29T16:21:17Z

retest this please

SparkQA · 2014-10-29T16:24:47Z

Test build #22456 has started for PR 2814 at commit 514cb38.

This patch merges cleanly.

SparkQA · 2014-10-29T18:06:42Z

Test build #22456 has finished for PR 2814 at commit 514cb38.

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

AmplabJenkins · 2014-10-29T18:06:46Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22456/
Test PASSed.

andrewor14 · 2014-10-29T23:28:02Z

LGTM. Looks like there are now merge conflicts.

….hadoop to org.apache.spark

…o part of the PairRDDFunctions type hierarchy though?

srowen · 2014-10-30T21:08:55Z

@andrewor14 Yep, rebased it.

SparkQA · 2014-10-30T21:14:53Z

Test build #22563 has started for PR 2814 at commit ead1115.

This patch merges cleanly.

SparkQA · 2014-10-30T22:53:55Z

Test build #22563 has finished for PR 2814 at commit ead1115.

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

AmplabJenkins · 2014-10-30T22:53:59Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22563/
Test PASSed.

andrewor14 · 2014-10-30T22:54:18Z

Ok I'm merging this. Thanks.

andrewor14 · 2014-10-31T00:57:57Z

Hey @srowen I reverted this because it was failing tests for Spark built with Hadoop 1.0.4. I haven't dug into it deeply but it seems to be failing at run time due to some reflection issue. Do you know what's going on?

[info] FileSuite:
2014-10-30 16:34:12.769 java[30516:5903] Unable to load realm info from SCDynamicStore
[info] - text files *** FAILED *** (1 second, 112 milliseconds)
[info]   java.lang.IllegalAccessException: Class org.apache.spark.mapred.SparkHadoopMapRedUtil$class can not access a member of class org.apache.hadoop.mapred.JobContext with modifiers ""
[info]   at sun.reflect.Reflection.ensureMemberAccess(Reflection.java:109)
[info]   at java.lang.reflect.AccessibleObject.slowCheckMemberAccess(AccessibleObject.java:261)
[info]   at java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:253)
[info]   at java.lang.reflect.Constructor.newInstance(Constructor.java:517)
[info]   at org.apache.spark.mapred.SparkHadoopMapRedUtil$class.newJobContext(SparkHadoopMapRedUtil.scala:29)
[info]   at org.apache.spark.SparkHadoopWriter.newJobContext(SparkHadoopWriter.scala:39)
[info]   at org.apache.spark.SparkHadoopWriter.getJobContext(SparkHadoopWriter.scala:149)
[info]   at org.apache.spark.SparkHadoopWriter.preSetup(SparkHadoopWriter.scala:63)
[info]   at org.apache.spark.rdd.PairRDDFunctions.saveAsHadoopDataset(PairRDDFunctions.scala:1033)
[info]   at org.apache.spark.rdd.PairRDDFunctions.saveAsHadoopFile(PairRDDFunctions.scala:939)
[info]   at org.apache.spark.rdd.PairRDDFunctions.saveAsHadoopFile(PairRDDFunctions.scala:848)
[info]   at org.apache.spark.rdd.RDD.saveAsTextFile(RDD.scala:1162)
[info]   at org.apache.spark.FileSuite$$anonfun$1.apply$mcV$sp(FileSuite.scala:53)
[info]   at org.apache.spark.FileSuite$$anonfun$1.apply(FileSuite.scala:49)
[info]   at org.apache.spark.FileSuite$$anonfun$1.apply(FileSuite.scala:49)

srowen · 2014-10-31T08:53:07Z

Hm, that's weird since I thought I ran the test against the default Hadoop, and that's 1.0.4. Which test failed? (I'll go look around jenkins too)

I can't find 1.0.4 source at the moment, but it looks like in ancient times, the JobContextImpl constructor was package-private:

http://grepcode.com/file/repo1.maven.org/maven2/org.apache.hadoop/hadoop-mapred/0.22.0/org/apache/hadoop/mapred/JobContextImpl.java

So JobContext is an interface and JobContextImpl has a non-public constructor. That's probably why this mechanism was created in the first place. Another solution is to change the constructor's accessibility at runtime. I can try that and verify the test again.

All of this would go away once Hadoop 1.x is not supported anymore too, along with some more acrobatics inside SparkHadoopWriter

…ot use package org.apache.hadoop andrewor14 Another try at SPARK-1209, to address #2814 (comment) I successfully tested with `mvn -Dhadoop.version=1.0.4 -DskipTests clean package; mvn -Dhadoop.version=1.0.4 test` I assume that is what failed Jenkins last time. I also tried `-Dhadoop.version1.2.1` and `-Phadoop-2.4 -Pyarn -Phive` for more coverage. So this is why the class was put in `org.apache.hadoop` to begin with, I assume. One option is to leave this as-is for now and move it only when Hadoop 1.0.x support goes away. This is the other option, which adds a call to force the constructor to be public at run-time. It's probably less surprising than putting Spark code in `org.apache.hadoop`, but, does involve reflection. A `SecurityManager` might forbid this, but it would forbid a lot of stuff Spark does. This would also only affect Hadoop 1.0.x it seems. Author: Sean Owen <sowen@cloudera.com> Closes #3048 from srowen/SPARK-1209 and squashes the following commits: 0d48f4b [Sean Owen] For Hadoop 1.0.x, make certain constructors public, which were public in later versions 466e179 [Sean Owen] Disable MIMA warnings resulting from moving the class -- this was also part of the PairRDDFunctions type hierarchy though? eb61820 [Sean Owen] Move SparkHadoopMapRedUtil / SparkHadoopMapReduceUtil from org.apache.hadoop to org.apache.spark (cherry picked from commit f8e5732) Signed-off-by: Patrick Wendell <pwendell@gmail.com>

…ot use package org.apache.hadoop andrewor14 Another try at SPARK-1209, to address #2814 (comment) I successfully tested with `mvn -Dhadoop.version=1.0.4 -DskipTests clean package; mvn -Dhadoop.version=1.0.4 test` I assume that is what failed Jenkins last time. I also tried `-Dhadoop.version1.2.1` and `-Phadoop-2.4 -Pyarn -Phive` for more coverage. So this is why the class was put in `org.apache.hadoop` to begin with, I assume. One option is to leave this as-is for now and move it only when Hadoop 1.0.x support goes away. This is the other option, which adds a call to force the constructor to be public at run-time. It's probably less surprising than putting Spark code in `org.apache.hadoop`, but, does involve reflection. A `SecurityManager` might forbid this, but it would forbid a lot of stuff Spark does. This would also only affect Hadoop 1.0.x it seems. Author: Sean Owen <sowen@cloudera.com> Closes #3048 from srowen/SPARK-1209 and squashes the following commits: 0d48f4b [Sean Owen] For Hadoop 1.0.x, make certain constructors public, which were public in later versions 466e179 [Sean Owen] Disable MIMA warnings resulting from moving the class -- this was also part of the PairRDDFunctions type hierarchy though? eb61820 [Sean Owen] Move SparkHadoopMapRedUtil / SparkHadoopMapReduceUtil from org.apache.hadoop to org.apache.spark

srowen changed the title ~~SPARK-1209 [CORE] SparkHadoopUtil should not use package org.apache.hadoop~~ SPARK-1209 [CORE] SparkHadoop{MapRed,MapReduce}Util should not use package org.apache.hadoop Oct 15, 2014

srowen force-pushed the SPARK-1209 branch from ec1b84a to dd603e2 Compare October 15, 2014 16:46

srowen force-pushed the SPARK-1209 branch from 821d12a to 11e7d5d Compare October 19, 2014 22:48

srowen force-pushed the SPARK-1209 branch from 11e7d5d to 514cb38 Compare October 26, 2014 18:32

srowen added 2 commits October 30, 2014 22:07

Move SparkHadoopMapRedUtil / SparkHadoopMapReduceUtil from org.apache…

2d42c1d

….hadoop to org.apache.spark

Disable MIMA warnings resulting from moving the class -- this was als…

ead1115

…o part of the PairRDDFunctions type hierarchy though?

srowen force-pushed the SPARK-1209 branch from 514cb38 to ead1115 Compare October 30, 2014 21:08

asfgit closed this in 68cb69d Oct 30, 2014

srowen mentioned this pull request Nov 1, 2014

SPARK-1209 [CORE] (Take 2) SparkHadoop{MapRed,MapReduce}Util should not use package org.apache.hadoop #3048

Closed

SPARK-1209 [CORE] SparkHadoop{MapRed,MapReduce}Util should not use package org.apache.hadoop #2814

SPARK-1209 [CORE] SparkHadoop{MapRed,MapReduce}Util should not use package org.apache.hadoop #2814

Uh oh!

Conversation

srowen commented Oct 15, 2014

Uh oh!

SparkQA commented Oct 15, 2014

Uh oh!

AmplabJenkins commented Oct 15, 2014

Uh oh!

shaneknapp commented Oct 15, 2014

Uh oh!

srowen commented Oct 15, 2014

Uh oh!

SparkQA commented Oct 15, 2014

Uh oh!

AmplabJenkins commented Oct 15, 2014

Uh oh!

AmplabJenkins commented Oct 15, 2014

Uh oh!

srowen commented Oct 16, 2014

Uh oh!

SparkQA commented Oct 16, 2014

Uh oh!

SparkQA commented Oct 16, 2014

Uh oh!

AmplabJenkins commented Oct 16, 2014

Uh oh!

SparkQA commented Oct 19, 2014

Uh oh!

SparkQA commented Oct 20, 2014

Uh oh!

AmplabJenkins commented Oct 20, 2014

Uh oh!

SparkQA commented Oct 26, 2014

Uh oh!

SparkQA commented Oct 26, 2014

Uh oh!

AmplabJenkins commented Oct 26, 2014

Uh oh!

andrewor14 commented Oct 28, 2014

Uh oh!

SparkQA commented Oct 28, 2014

Uh oh!

SparkQA commented Oct 28, 2014

Uh oh!

AmplabJenkins commented Oct 28, 2014

Uh oh!

andrewor14 commented Oct 28, 2014

Uh oh!

SparkQA commented Oct 28, 2014

Uh oh!

SparkQA commented Oct 29, 2014

Uh oh!

AmplabJenkins commented Oct 29, 2014

Uh oh!

andrewor14 commented Oct 29, 2014

Uh oh!

SparkQA commented Oct 29, 2014

Uh oh!

SparkQA commented Oct 29, 2014

Uh oh!

AmplabJenkins commented Oct 29, 2014

Uh oh!

andrewor14 commented Oct 29, 2014

Uh oh!

srowen commented Oct 30, 2014

Uh oh!

SparkQA commented Oct 30, 2014

Uh oh!

SparkQA commented Oct 30, 2014

Uh oh!

AmplabJenkins commented Oct 30, 2014

Uh oh!

andrewor14 commented Oct 30, 2014

Uh oh!

andrewor14 commented Oct 31, 2014

Uh oh!

srowen commented Oct 31, 2014

Uh oh!