[SPARK-19810][BUILD][CORE] Remove support for Scala 2.10 #17150

srowen · 2017-03-03T12:44:18Z

What changes were proposed in this pull request?

Remove Scala 2.10 build profiles and support
Replace some 2.10 support in scripts with commented placeholders for 2.12 later
Remove deprecated API calls from 2.10 support
Remove usages of deprecated context bounds where possible
Remove Scala 2.10 workarounds like ScalaReflectionLock
Other minor Scala warning fixes

How was this patch tested?

Existing tests

srowen · 2017-03-03T12:44:35Z

R/pkg/DESCRIPTION

(Will fix this -- need to probably make the build stop changing this)

srowen · 2017-03-03T12:45:34Z

core/src/main/scala/org/apache/spark/SparkContext.scala

This and the SequenceFileRDDFunctions change are the only questionable ones. This should be the equivalent declaration that context bounds desugar into, but the question is whether this actually induces an obscure API change.

srowen · 2017-03-03T12:45:50Z

core/src/main/scala/org/apache/spark/ui/JettyUtils.scala

These didn't need to be context bounds in the first place

srowen · 2017-03-03T12:46:08Z

dev/change-version-to-2.11.sh

These scripts were deprecated wrappers

SparkQA · 2017-03-03T13:00:58Z

Test build #73843 has finished for PR 17150 at commit 933d180.

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

srowen · 2017-03-03T13:11:31Z

The context bound changes won't work on the two public methods:

[error]  * deprecated method accumulableCollection(java.lang.Object,scala.Function1,scala.reflect.ClassTag)org.apache.spark.Accumulable in class org.apache.spark.SparkContext's type is different in current version, where it is (java.lang.Object,scala.reflect.ClassTag,scala.Function1)org.apache.spark.Accumulable instead of (java.lang.Object,scala.Function1,scala.reflect.ClassTag)org.apache.spark.Accumulable
[error]    filter with: ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.SparkContext.accumulableCollection")
[error]  * method this(org.apache.spark.rdd.RDD,java.lang.Class,java.lang.Class,scala.Function1,scala.reflect.ClassTag,scala.Function1,scala.reflect.ClassTag)Unit in class org.apache.spark.rdd.SequenceFileRDDFunctions's type is different in current version, where it is (org.apache.spark.rdd.RDD,java.lang.Class,java.lang.Class,scala.reflect.ClassTag,scala.reflect.ClassTag,scala.Function1,scala.Function1)Unit instead of (org.apache.spark.rdd.RDD,java.lang.Class,java.lang.Class,scala.Function1,scala.reflect.ClassTag,scala.Function1,scala.reflect.ClassTag)Unit
[error]    filter with: ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.rdd.SequenceFileRDDFunctions.this")

Quite unfortunately, they generate signatures that take the same arguments but in differing orders. This bit is one of those tiny breaking changes that will have to wait for Spark 3.x. As I recall they still work in Scala 2.12, even though they are deprecated.

SparkQA · 2017-03-04T12:19:07Z

Test build #73899 has finished for PR 17150 at commit 1d12539.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class SequenceFileRDDFunctions[K <% Writable: ClassTag, V <% Writable : ClassTag](

SparkQA · 2017-03-05T14:38:29Z

Test build #73930 has finished for PR 17150 at commit fe61270.

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

srowen · 2017-03-06T10:29:46Z

bin/load-spark-env.cmd

I commented bits like this out, but updated it as well for when 2.12 needs to be supported as an alternative

srowen · 2017-03-06T10:30:18Z

dev/create-release/release-build.sh

This was redundant with the line 8 lines above

SparkQA · 2017-03-06T11:44:38Z

Test build #73994 has finished for PR 17150 at commit a42de48.

This patch fails Python style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-03-06T11:44:39Z

Test build #3596 has finished for PR 17150 at commit a42de48.

This patch fails Python style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-03-06T14:27:33Z

Test build #73999 has finished for PR 17150 at commit beb1fdf.

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

SparkQA · 2017-03-06T18:52:39Z

Test build #3597 has finished for PR 17150 at commit beb1fdf.

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

SparkQA · 2017-04-11T14:49:46Z

Test build #75707 has finished for PR 17150 at commit cccfbdf.

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

SparkQA · 2017-05-06T19:53:54Z

Test build #76525 has finished for PR 17150 at commit d255491.

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

SparkQA · 2017-05-20T14:41:24Z

Test build #77124 has finished for PR 17150 at commit 13e1d7b.

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

SparkQA · 2017-05-21T12:30:49Z

Test build #77145 has finished for PR 17150 at commit f4ff410.

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

SparkQA · 2017-05-23T15:54:00Z

Test build #77251 has finished for PR 17150 at commit 5794eb1.

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

SparkQA · 2017-05-23T19:53:59Z

Test build #77260 has finished for PR 17150 at commit 080473e.

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

SparkQA · 2017-05-24T01:29:09Z

Test build #77265 has finished for PR 17150 at commit 7a30de4.

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

SparkQA · 2017-05-24T06:43:51Z

Test build #3752 has started for PR 17150 at commit 7a30de4.

SparkQA · 2017-05-25T12:34:51Z

Test build #3754 has finished for PR 17150 at commit 7a30de4.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2017-05-25T15:31:17Z

Test build #3756 has finished for PR 17150 at commit 7a30de4.

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

jiangxb1987 · 2017-06-20T14:38:37Z

gently ping @srowen

srowen · 2017-06-20T15:47:52Z

@jiangxb1987 this can't be merges until 2.2 is released. It is open on purpose.

fbascheper · 2017-07-12T08:25:45Z

I just noticed that spark 2.2.0 was released yesterday. Is this the right time to move on with this PR?

dongjoon-hyun

+1, LGTM, too.

ueshin · 2017-07-13T04:08:02Z

I guess we can also remove thread safety tests at the bottom of org.apache.spark.sql.catalyst.ScalaReflectionSuite (ScalaReflectionSuite.scala#L341-L376).

cloud-fan · 2017-07-13T08:52:57Z

core/src/main/scala/org/apache/spark/SparkContext.scala

  def accumulableCollection[R <% Growable[T] with TraversableOnce[T] with Serializable: ClassTag, T]
      (initialValue: R): Accumulable[R, T] = {
+    // TODO the context bound (<%) above should be replaced with simple type bound and implicit
+    // conversion but is a breaking change. This should be fixed in Spark 3.x.


so context bound is not recommended in scala?

It generates a warning in scala 2.11, yeah

cloud-fan · 2017-07-13T09:04:22Z

sql/core/src/main/scala/org/apache/spark/sql/execution/python/ExtractPythonUDFs.scala

        val validUdfs = udfs.filter { udf =>
          // Check to make sure that the UDF can be evaluated with only the input of this child.
          udf.references.subsetOf(child.outputSet)
-        }.toArray  // Turn it into an array since iterators cannot be serialized in Scala 2.10


can we remove the toArray?

Probably, but I stayed conservative here. At least the comment is obsolete

cloud-fan · 2017-07-13T09:07:29Z

LGTM, merging to master! we can address remaining comments in follow-up PR. Thanks for working on it!

srowen · 2017-07-13T09:09:22Z

OK, I think we could have just made the suggested changes here too, but I suspect we will find a thing or two we have to fix up along the way anyway.

cloud-fan · 2017-07-13T09:11:43Z

BTW don't forget this comment: #17150 (comment)

I went ahead merging this because the scala 2.10 build was broken :)

srowen · 2017-07-13T09:50:22Z

BTW @cloud-fan @JoshRosen can I delete the master 2.10 compile Jenkins jobs? they're going to fail from here on out.

https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Compile/job/spark-master-compile-maven-scala-2.10/

(Not sure I have permissions to do it though.)

JoshRosen · 2017-07-13T15:27:26Z

@srowen, I'll disable those master branch 2.10 jobs and will update scripts to prevent them from being redeployed by accident.

cloud-fan · 2017-07-14T01:12:18Z

@srowen @JoshRosen After thinking more about it, will this PR make it harder to backport bug fixes PRs to the previous branch? I did a little search and seems we broke scala 2.10 a little frequent before: https://github.com/apache/spark/pulls?utf8=%E2%9C%93&q=is%3Apr%20is%3Aclosed%202.10%20in%3Atitle

srowen · 2017-07-14T07:16:11Z

Do you mean, it'll be harder to know if a back-port broke the 2.10 build? yes. I don't think we have a choice though. We would detect the breakage eventually, and backports become rare over time.

rxin · 2017-07-14T18:25:48Z

Maybe do it a bit later, when the backport rate drops? E.g. it's unlikely we still do a lot of backports when 2.3 is cut.

srowen · 2017-07-14T18:30:03Z

Do what later?
BTW there are still jobs that compile vs 2.10 for 2.2, 2.1, etc

rxin · 2017-07-14T18:33:49Z

Do the removal (i.e. this PR).

srowen · 2017-07-14T18:40:00Z

This is already merged though. This doesn't seem like a reason to revert -- there's not even an actual problem here. It's the same potential problem we've always had, that 2.10 breakage might only be caught by the Jenkins job. 2.10 breaks were not really common, all the less so for back-ports. I pledge to watch the compile jobs, if that helps. But we really need to get on to 2.12 support now.

rxin · 2017-07-14T18:47:04Z

Are you working on 2.12?

srowen · 2017-07-14T18:51:51Z

I do have a local branch with, I think, most of the key changes. I do want to unblock that work. But that's a little separate. Really I'm just narrowly arguing here that this change doesn't make it meaningfully harder to keep 2.10 working in older branches. It's no harder than it was previously, and I'm happy to keep an eye on it if anyone's concerned.

…e Scala 2.10 ## What changes were proposed in this pull request? Follow up to a few comments on #17150 (comment) that couldn't be addressed before it was merged. ## How was this patch tested? Existing tests. Author: Sean Owen <[email protected]> Closes #18646 from srowen/SPARK-19810.2.

HyukjinKwon · 2017-07-21T09:17:52Z

Hi all, I did not open a JIRA because I am not very sure if it is an issue and it sounds maybe my environment problem.

However, I believe it is worth leaving a note. The build with Maven is fine (./build/mvn clean -DskipTests package as described in https://spark.apache.org/docs/latest/building-spark.html#buildmvn) but I am seeing the error with SBT after this commit. The build itself succeeds but it fails to run ./bin/spark-shell. I tried the command lines as below (as described in http://spark.apache.org/developer-tools.html):

./build/sbt clean package

./bin/spark-shell

Exception in thread "main" java.lang.NoClassDefFoundError: org/apache/commons/logging/LogFactory
	at org.apache.hadoop.conf.Configuration.<clinit>(Configuration.java:173)
	at org.apache.spark.deploy.SparkSubmit$.prepareSubmitEnvironment(SparkSubmit.scala:340)
	at org.apache.spark.deploy.SparkSubmit$.submit(SparkSubmit.scala:157)
	at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:123)
	at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala)
Caused by: java.lang.ClassNotFoundException: org.apache.commons.logging.LogFactory
	at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	... 5 more

I could reproduce this with SBT on ...

CentOS Linux release 7.3.1611 (Core) / Java(TM) SE Runtime Environment (build 1.8.0_101-b13)
macOS 10.12.3 (16D32) / Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Ubuntu 14.04 LTS / Java(TM) SE Runtime Environment (build 1.8.0_131-b11)

I checked this is fine with Maven is fine on ...

CentOS Linux release 7.3.1611 (Core) / Java(TM) SE Runtime Environment (build 1.8.0_101-b13)
macOS 10.12.3 (16D32) / Java(TM) SE Runtime Environment (build 1.8.0_45-b14)

I identified this commit with SBT before/after the commit on ...

macOS 10.12.3 (16D32) / Java(TM) SE Runtime Environment (build 1.8.0_45-b14)

It looks this one is introduced here.

Could anyone double check this and see if I am mistaken? I just wonder if the same commands are consistently being failed on other environments (rather than the exact failure cause).

srowen · 2017-07-21T09:36:07Z

Yeah this change did change some dependencies, and it's possible something happened to have commons-logging included before and now it doesn't, and we need an explicit dependency somewhere. I'll look into it, as it ought to be pretty easy to patch if there's a gap here.

srowen · 2017-07-21T10:06:10Z

pom.xml

        <artifactId>jcl-over-slf4j</artifactId>
        <version>${slf4j.version}</version>
-        <!-- <scope>runtime</scope> --> <!-- more correct, but scalac 2.10.3 doesn't like it -->
+        <scope>runtime</scope>


@HyukjinKwon it was this change. I think we need to retain it to keep commons-logging (well, the SLF4J shim for it) on the classpath. SBT isn't including it in the build somehow even though it's runtime scope, which is really correct. I think we've seen this with the Maven-SBT plugin before.

… compile scope for SBT build ## What changes were proposed in this pull request? jcl-over-slf4j dependency needs to be compile scope for SBT build, to make it available for commons-logging dependents like Hadoop apache#17150 (comment) https://github.com/apache/spark/pull/17150/files#r128728089 ## How was this patch tested? Manual tests Author: Sean Owen <[email protected]> Closes apache#18703 from srowen/SPARK-19810.2.

srowen commented Mar 3, 2017

View reviewed changes

srowen force-pushed the SPARK-19810 branch from fe61270 to a42de48 Compare March 6, 2017 10:27

srowen changed the title ~~[SPARK-19810][WIP][BUILD][CORE] Remove support for Scala 2.10~~ [SPARK-19810][BUILD][CORE] Remove support for Scala 2.10 Mar 6, 2017

srowen commented Mar 6, 2017

View reviewed changes

srowen force-pushed the SPARK-19810 branch from beb1fdf to cccfbdf Compare April 11, 2017 11:38

srowen force-pushed the SPARK-19810 branch from cccfbdf to d255491 Compare May 6, 2017 16:49

srowen force-pushed the SPARK-19810 branch from d255491 to 13e1d7b Compare May 20, 2017 11:32

srowen force-pushed the SPARK-19810 branch from f4ff410 to 5794eb1 Compare May 23, 2017 13:28

srowen force-pushed the SPARK-19810 branch from 080473e to 7a30de4 Compare May 23, 2017 23:07

dongjoon-hyun approved these changes Jul 13, 2017

View reviewed changes

cloud-fan reviewed Jul 13, 2017

View reviewed changes

asfgit closed this in 425c4ad Jul 13, 2017

srowen deleted the SPARK-19810 branch July 14, 2017 08:49

srowen mentioned this pull request Jul 16, 2017

[SPARK-19810][MINOR][FOLLOW-UP] Follow-ups from SPARK-19810 to remove Scala 2.10 #18646

Closed

HyukjinKwon mentioned this pull request Jul 21, 2017

[SPARK-21485][SQL][DOCS] Spark SQL documentation generation for built-in functions #18702

Closed

srowen commented Jul 21, 2017

View reviewed changes

srowen mentioned this pull request Jul 21, 2017

[SPARK-19810][BUILD][FOLLOW-UP] jcl-over-slf4j dependency needs to be compile scope for SBT build #18703

Closed

[SPARK-19810][BUILD][CORE] Remove support for Scala 2.10 #17150

[SPARK-19810][BUILD][CORE] Remove support for Scala 2.10 #17150

Uh oh!

Conversation

srowen commented Mar 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 3, 2017

Uh oh!

srowen commented Mar 3, 2017

Uh oh!

SparkQA commented Mar 4, 2017

Uh oh!

SparkQA commented Mar 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 6, 2017

Uh oh!

SparkQA commented Mar 6, 2017

Uh oh!

SparkQA commented Mar 6, 2017

Uh oh!

SparkQA commented Mar 6, 2017

Uh oh!

SparkQA commented Apr 11, 2017

Uh oh!

SparkQA commented May 6, 2017

Uh oh!

SparkQA commented May 20, 2017

Uh oh!

SparkQA commented May 21, 2017

Uh oh!

SparkQA commented May 23, 2017

Uh oh!

SparkQA commented May 23, 2017

Uh oh!

SparkQA commented May 24, 2017

Uh oh!

SparkQA commented May 24, 2017

Uh oh!

SparkQA commented May 25, 2017

Uh oh!

SparkQA commented May 25, 2017

Uh oh!

jiangxb1987 commented Jun 20, 2017

Uh oh!

srowen commented Jun 20, 2017

Uh oh!

fbascheper commented Jul 12, 2017

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

ueshin commented Jul 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Jul 13, 2017

Uh oh!

srowen commented Jul 13, 2017

Uh oh!

srowen commented Mar 3, 2017 •

edited

Loading

cloud-fan commented Jul 13, 2017 •

edited

Loading

HyukjinKwon commented Jul 21, 2017 •

edited

Loading