Skip to content
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.

Sign up for GitHub

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-24418][Build] Upgrade Scala to 2.11.12 and 2.12.6 #21495

Closed
wants to merge 8 commits into from
Closed

[SPARK-24418][Build] Upgrade Scala to 2.11.12 and 2.12.6 #21495

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
de790fd
Upgrade scala to 2.11.12 and 2.12.6
dbtsai May 29, 2018
f91d75a
Update LICENSE
dbtsai Jun 8, 2018
c1ffd0b
Update JLine
dbtsai Jun 8, 2018
631ef48
update scala-parser-combinators
dbtsai Jun 8, 2018
96e87c2
Error handling
dbtsai Jun 8, 2018
4c852fa
Fix compilation
dbtsai Jun 8, 2018
096e6a9
temp
dbtsai Jun 9, 2018
82ca5f6
Revert hack
dbtsai Jun 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions LICENSE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,11 @@ The text of each license is also included at licenses/LICENSE-[project].txt.
(Interpreter classes (all .scala files in repl/src/main/scala
except for Main.Scala, SparkHelper.scala and ExecutorClassLoader.scala),
and for SerializableMapWrapper in JavaUtils.scala)
(BSD-like) Scala Actors library (org.scala-lang:scala-actors:2.11.8 - http://www.scala-lang.org/)
(BSD-like) Scala Compiler (org.scala-lang:scala-compiler:2.11.8 - http://www.scala-lang.org/)
(BSD-like) Scala Compiler (org.scala-lang:scala-reflect:2.11.8 - http://www.scala-lang.org/)
(BSD-like) Scala Library (org.scala-lang:scala-library:2.11.8 - http://www.scala-lang.org/)
(BSD-like) Scalap (org.scala-lang:scalap:2.11.8 - http://www.scala-lang.org/)
(BSD-like) Scala Actors library (org.scala-lang:scala-actors:2.11.12 - http://www.scala-lang.org/)
(BSD-like) Scala Compiler (org.scala-lang:scala-compiler:2.11.12 - http://www.scala-lang.org/)
(BSD-like) Scala Compiler (org.scala-lang:scala-reflect:2.11.12 - http://www.scala-lang.org/)
(BSD-like) Scala Library (org.scala-lang:scala-library:2.11.12 - http://www.scala-lang.org/)
(BSD-like) Scalap (org.scala-lang:scalap:2.11.12 - http://www.scala-lang.org/)
(BSD-style) scalacheck (org.scalacheck:scalacheck_2.11:1.10.0 - http://www.scalacheck.org)
(BSD-style) spire (org.spire-math:spire_2.11:0.7.1 - http://spire-math.org)
(BSD-style) spire-macros (org.spire-math:spire-macros_2.11:0.7.1 - http://spire-math.org)
Expand Down
6 changes: 3 additions & 3 deletions dev/deps/spark-deps-hadoop-2.6
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,10 @@ parquet-jackson-1.10.0.jar
protobuf-java-2.5.0.jar
py4j-0.10.7.jar
pyrolite-4.13.jar
scala-compiler-2.11.8.jar
scala-library-2.11.8.jar
scala-compiler-2.11.12.jar
scala-library-2.11.12.jar
scala-parser-combinators_2.11-1.0.4.jar
scala-reflect-2.11.8.jar
scala-reflect-2.11.12.jar
scala-xml_2.11-1.0.5.jar
shapeless_2.11-2.3.2.jar
slf4j-api-1.7.16.jar
Expand Down
6 changes: 3 additions & 3 deletions dev/deps/spark-deps-hadoop-2.7
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ parquet-jackson-1.10.0.jar
protobuf-java-2.5.0.jar
py4j-0.10.7.jar
pyrolite-4.13.jar
scala-compiler-2.11.8.jar
scala-library-2.11.8.jar
scala-compiler-2.11.12.jar
scala-library-2.11.12.jar
scala-parser-combinators_2.11-1.0.4.jar
scala-reflect-2.11.8.jar
scala-reflect-2.11.12.jar
scala-xml_2.11-1.0.5.jar
shapeless_2.11-2.3.2.jar
slf4j-api-1.7.16.jar
Expand Down
6 changes: 3 additions & 3 deletions dev/deps/spark-deps-hadoop-3.1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,10 @@ protobuf-java-2.5.0.jar
py4j-0.10.7.jar
pyrolite-4.13.jar
re2j-1.1.jar
scala-compiler-2.11.8.jar
scala-library-2.11.8.jar
scala-compiler-2.11.12.jar
scala-library-2.11.12.jar
scala-parser-combinators_2.11-1.0.4.jar
scala-reflect-2.11.8.jar
scala-reflect-2.11.12.jar
scala-xml_2.11-1.0.5.jar
shapeless_2.11-2.3.2.jar
slf4j-api-1.7.16.jar
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
<commons.math3.version>3.4.1</commons.math3.version>
<!-- managed up from 3.2.1 for SPARK-11652 -->
<commons.collections.version>3.2.2</commons.collections.version>
<scala.version>2.11.8</scala.version>
<scala.version>2.11.12</scala.version>
<scala.binary.version>2.11</scala.binary.version>
<codehaus.jackson.version>1.9.13</codehaus.jackson.version>
<fasterxml.jackson.version>2.6.7</fasterxml.jackson.version>
Expand Down Expand Up @@ -2748,7 +2748,7 @@
<profile>
<id>scala-2.12</id>
<properties>
<scala.version>2.12.4</scala.version>
<scala.version>2.12.6</scala.version>
<scala.binary.version>2.12</scala.binary.version>
</properties>
<build>
Expand Down
21 changes: 4 additions & 17 deletions repl/scala-2.11/src/main/scala/org/apache/spark/repl/SparkILoop.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class SparkILoop(in0: Option[BufferedReader], out: JPrintWriter)
def this() = this(None, new JPrintWriter(Console.out, true))

override def createInterpreter(): Unit = {
intp = new SparkILoopInterpreter(settings, out)
intp = new SparkILoopInterpreter(settings, out, initializeSpark)
}

val initializationCommands: Seq[String] = Seq(
Expand Down Expand Up @@ -73,12 +73,9 @@ class SparkILoop(in0: Option[BufferedReader], out: JPrintWriter)
"import org.apache.spark.sql.functions._"
)

def initializeSpark() {
intp.beQuietDuring {
savingReplayStack { // remove the commands from session history.
initializationCommands.foreach(processLine)
}
}
def initializeSpark(): Unit = savingReplayStack {
// `savingReplayStack` removes the commands from session history.
initializationCommands.foreach(intp quietRun _)
}

/** Print a welcome message */
Expand All @@ -101,16 +98,6 @@ class SparkILoop(in0: Option[BufferedReader], out: JPrintWriter)
/** Available commands */
override def commands: List[LoopCommand] = standardCommands

/**
* We override `loadFiles` because we need to initialize Spark *before* the REPL
* sees any files, so that the Spark context is visible in those files. This is a bit of a
* hack, but there isn't another hook available to us at this point.
*/
override def loadFiles(settings: Settings): Unit = {
initializeSpark()
super.loadFiles(settings)
}

override def resetCommand(line: String): Unit = {
super.resetCommand(line)
initializeSpark()
Expand Down
18 changes: 16 additions & 2 deletions repl/scala-2.11/src/main/scala/org/apache/spark/repl/SparkILoopInterpreter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,22 @@ import scala.collection.mutable
import scala.tools.nsc.Settings
import scala.tools.nsc.interpreter._

class SparkILoopInterpreter(settings: Settings, out: JPrintWriter) extends IMain(settings, out) {
self =>
class SparkILoopInterpreter(settings: Settings, out: JPrintWriter, initializeSpark: () => Unit)
extends IMain(settings, out) { self =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: two spaces

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought for extends, it's four spaces?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, four spaces is OK.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's definitely two spaces after a period. I've been wanting to make that joke, but held off.

Copy link
Member

@HyukjinKwon HyukjinKwon Jun 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys, parameters are listed in 4 spaces and other keywords after that are lined up with 2 spaces, which is written in https://github.com/databricks/scala-style-guide#spacing-and-indentation

In case of two lines, it's not explicitly written but wouldn't we better stick to the example as possible as we can? "Use 2-space indentation in general.".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there's a bunch of code in Spark where both 4-space and 2-space indentation are existed. From my thought it is not so worthy to stick to this tiny point.

If you want to fix it, then creating a separate PR to fix all of them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can fix them in place. There are many other style nits too. It's not worth swiping them making the backport harder.

I am fine with ignoring such nits and they don't block this PR but it's not something we should say the opposite side is okay.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really not worthy to block on this thing (if it is the only issue).


/**
* We override `initializeSynchronous` to initialize Spark *after* `intp` is properly initialized
* and *before* the REPL sees any files in the private `loadInitFiles` functions, so that
* the Spark context is visible in those files.
*
* This is a bit of a hack, but there isn't another hook available to us at this point.
*
* See the discussion in Scala community https://github.com/scala/bug/issues/10913 for detail.
*/
override def initializeSynchronous(): Unit = {
super.initializeSynchronous()
initializeSpark()
Copy link
Member Author

@dbtsai dbtsai Jun 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@som-snytt It's working, but I'm wondering if I'm doing it correctly.

In this case, I'll use $intp without checking

if (intp.reporter.hasErrors) {
  echo("Interpreter encountered errors during initialization!")
  null
}

in iLoop.scala. Of course, I can add the checking in our own code, but it doesn't look right to me.

And intp.quietBind(NamedParam[IMain]("$intp", intp)(tagOfIMain, classTag[IMain])) will not be executed before our custom Spark initialization code.

Copy link
Member

@dongjoon-hyun dongjoon-hyun Jun 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my environment (with build/sbt), I'm hitting NoSuchMethodError like the following. Did you see something like this?

~/PR-21495:PR-21495$ bin/spark-shell
18/06/07 23:39:00 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
Using Spark's default log4j profile: org/apache/spark/log4j-defaults.properties
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
Welcome to
      ____              __
     / __/__  ___ _____/ /__
    _\ \/ _ \/ _ `/ __/  '_/
   /___/ .__/\_,_/_/ /_/\_\   version 2.4.0-SNAPSHOT
      /_/

Using Scala version 2.11.12 (OpenJDK 64-Bit Server VM, Java 1.8.0_171)
Type in expressions to have them evaluated.
Type :help for more information.

scala> Spark context Web UI available at http://localhost:4040
Spark context available as 'sc' (master = local[*], app id = local-1528414746558).
Spark session available as 'spark'.
Exception in thread "main" java.lang.NoSuchMethodError: jline.console.completer.CandidateListCompletionHandler.setPrintSpaceAfterFullCompletion(Z)V
        at scala.tools.nsc.interpreter.jline.JLineConsoleReader.initCompletion(JLineReader.scala:139)
        at scala.tools.nsc.interpreter.jline.InteractiveReader.postInit(JLineReader.scala:54)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like jline version mismatch.

  • Spark 2.4.0-SNAPSHOT uses 2.12.1
  • Scala 2.11.12 uses 2.14.3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is working for me. I got scala-compiler-2.11.12.jar in my classpath. Can you do a clean build?

Copy link
Member

@dongjoon-hyun dongjoon-hyun Jun 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Of course, it's a clean clone and build of this PR. Accoring to the error message and the followings, we need jline-2.14.3.jar because scala uses the setPrintSpaceAfterFullCompletion API of higher version of jline. Could you confirm this, @som-snytt ?

$ java -version
openjdk version "1.8.0_171"
OpenJDK Runtime Environment (build 1.8.0_171-8u171-b11-0ubuntu0.18.04.1-b11)
OpenJDK 64-Bit Server VM (build 25.171-b11, mixed mode)

$ javap -cp jline-2.12.1.jar jline.console.completer.CandidateListCompletionHandler
Compiled from "CandidateListCompletionHandler.java"
public class jline.console.completer.CandidateListCompletionHandler implements jline.console.completer.CompletionHandler {
  public jline.console.completer.CandidateListCompletionHandler();
  public boolean complete(jline.console.ConsoleReader, java.util.List<java.lang.CharSequence>, int) throws java.io.IOException;
  public static void setBuffer(jline.console.ConsoleReader, java.lang.CharSequence, int) throws java.io.IOException;
  public static void printCandidates(jline.console.ConsoleReader, java.util.Collection<java.lang.CharSequence>) throws java.io.IOException;
}

$ javap -cp jline-2.14.3.jar  jline.console.completer.CandidateListCompletionHandler
Compiled from "CandidateListCompletionHandler.java"
public class jline.console.completer.CandidateListCompletionHandler implements jline.console.completer.CompletionHandler {
  public jline.console.completer.CandidateListCompletionHandler();
  public boolean getPrintSpaceAfterFullCompletion();
  public void setPrintSpaceAfterFullCompletion(boolean);
  public boolean isStripAnsi();
  public void setStripAnsi(boolean);
  public boolean complete(jline.console.ConsoleReader, java.util.List<java.lang.CharSequence>, int) throws java.io.IOException;
  public static void setBuffer(jline.console.ConsoleReader, java.lang.CharSequence, int) throws java.io.IOException;
  public static void printCandidates(jline.console.ConsoleReader, java.util.Collection<java.lang.CharSequence>) throws java.io.IOException;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we upgrade to the corresponding jline version together in this PR, @dbtsai ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completion was upgraded since the old days; also, other bugs required updating jline. There is interest in upgrading to jline 3.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for confirming, @som-snytt .

Copy link
Member Author

@dbtsai dbtsai Jun 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I know why it works for me.

I just run the following without building a distribution. jline was automatically resolved to newer version.

./build/sbt clean package
./bin/spark-shell

}

override lazy val memberHandlers = new {
val intp: self.type = self
Expand Down