Skip to content

[SPARK-13294] [PROJECT INFRA] Remove MiMa's dependency on spark-class / Spark assembly #11178

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

Closed
JoshRosen wants to merge 25 commits into from
Closed

[SPARK-13294] [PROJECT INFRA] Remove MiMa's dependency on spark-class / Spark assembly #11178

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
73b1550
[BUILD] Remove assembly/assembly step from dev/run-tests.
JoshRosen Feb 12, 2016
b6f1ce8
Also remove from Maven build step.
JoshRosen Feb 12, 2016
5528c48
Set SPARK_PREPEND_CLASSES in dev/mima.
JoshRosen Feb 12, 2016
bef62eb
Use 1 instead of true
JoshRosen Feb 12, 2016
76a365e
More attempts towards fixing MiMa.
JoshRosen Feb 12, 2016
9fc0f7a
Fix heap sizing.
JoshRosen Feb 13, 2016
31854eb
Try using classutil to find Spark classes.
JoshRosen Feb 13, 2016
906d8c8
Fix PySpark tests by setting SPARK_DIST_CLASSPATH
JoshRosen Feb 17, 2016
1f995a4
Merge remote-tracking branch 'origin/master' into remove-assembly-in-…
JoshRosen Mar 1, 2016
902b1b7
Update to fix classpaths in MiMA.
JoshRosen Mar 1, 2016
a73118e
Update to reflect #11426
JoshRosen Mar 1, 2016
64330d6
Second shot at fixing MiMa
JoshRosen Mar 2, 2016
9a122ad
Merge remote-tracking branch 'origin/master' into remove-assembly-in-…
JoshRosen Mar 10, 2016
db8e532
Update AbstractCommandBuilder to reflect recent relocations
JoshRosen Mar 10, 2016
cd7eb04
Just grab fullClasspath from assembly in MiMa, since we only run MiMA…
JoshRosen Mar 10, 2016
4756a1e
Remove now-unused o.a.s.tools code from SparkClassCommandBuilder
JoshRosen Mar 10, 2016
ae6b002
Fix help command of ./python/run-tests
JoshRosen Mar 10, 2016
dae4725
Bump to working version of sbt-dependency-graph to aid debugging
JoshRosen Mar 10, 2016
373fd52
Roll back non-MiMa-related changes (they'll go in later).
JoshRosen Mar 11, 2016
8ec1cc4
Revert "Bump to working version of sbt-dependency-graph to aid debugg…
JoshRosen Mar 11, 2016
5d32e74
Continue to build assembly for tests, but skip it before MiMa.
JoshRosen Mar 11, 2016
97b5d78
Revert attempts at improving MiMa code to minimize diff (will add bac…
JoshRosen Mar 11, 2016
f9e2b42
Turns out that we do need transitive deps.
JoshRosen Mar 11, 2016
4070c0d
Merge remote-tracking branch 'origin/master' into remove-assembly-in-…
JoshRosen Mar 11, 2016
86cd513
We can't use .tree.tpe due to Scala 2.10.
JoshRosen Mar 11, 2016
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
Prev Previous commit
Next Next commit
Update to fix classpaths in MiMA.
  • Loading branch information
@JoshRosen
JoshRosen committed Mar 1, 2016
commit 902b1b79f8ccfc2eaf1d4b25be64e947250665b1
47 changes: 42 additions & 5 deletions dev/mima
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,64 @@ set -e
FWDIR="$(cd "`dirname "$0"`"/..; pwd)"
cd "$FWDIR"

echo -e "q\n" | build/sbt oldDeps/update
rm -f .generated-mima*

# All default projects except for assemblies:
SPARK_PROJECTS=(
"common/network-common"
"common/network-shuffle"
"common/network-yarn"
"common/sketch"
"core"
"docker-integration-tests"
"examples"
"external/akka"
"external/flume"
"external/flume-sink"
"external/kafka"
"external/mqtt"
"external/twitter"
"external/zeromq"
"extras/kinesis-asl"
Copy link
Member

@srowen srowen Mar 10, 2016

Choose a reason for hiding this comment

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

BTW extras/* is now in external/, and docker* directories are also in external/

Copy link
Contributor Author

@JoshRosen JoshRosen Mar 10, 2016

Choose a reason for hiding this comment

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

It turns out that we don't support MiMa for the Maven build anyways, so I can just safely pull this in from one of SBT's fullClasspath outputs.

"extras/spark-ganglia-lgpl"
"graphx"
"launcher"
"mllib"
"repl"
"sql/catalyst"
"sql/core"
"sql/hive"
"sql/hive-thriftserver"
"streaming"
"tags"
"tools"
"unsafe"
"yarn"
)
SPARK_CLASSES=""
for project in "${SPARK_PROJECTS[@]}"; do
SPARK_CLASSES="$FWDIR/$project/target/scala-2.11/classes/:$SPARK_CLASSES"
done

TOOLS_CLASSPATH="$(build/sbt "export tools/fullClasspath" | tail -n1)"

rm -f .generated-mima*

# Generate Mima Ignore is called twice, first with latest built jars
# on the classpath and then again with previous version jars on the classpath.
# Because of a bug in GenerateMIMAIgnore that when old jars are ahead on classpath
# it did not process the new classes (which are in assembly jar).

generate_mima_ignore() {
echo "$TOOLS_CLASSPATH:$1"
java \
-XX:MaxPermSize=1g \
-Xmx2g \
-cp "$TOOLS_CLASSPATH:$1" \
org.apache.spark.tools.GenerateMIMAIgnore
}

generate_mima_ignore "$(build/sbt "export assembly/managedClasspath" | tail -n1)"
generate_mima_ignore "$(build/sbt "export oldDeps/managedClasspath" | tail -n1)"
SPARK_PROFILES="-Pyarn -Pspark-ganglia-lgpl -Pkinesis-asl -Phive-thriftserver -Phive"
generate_mima_ignore "$SPARK_CLASSES:$(build/sbt $SPARK_PROFILES "export assembly/managedClasspath" | tail -n1)"
generate_mima_ignore "$(build/sbt $SPARK_PROFILES "export oldDeps/managedClasspath" | tail -n1)"

echo -e "q\n" | build/sbt mima-report-binary-issues | grep -v -e "info.*Resolving"
ret_val=$?
Expand Down
14 changes: 12 additions & 2 deletions launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ List<String> buildClassPath(String appClassPath) throws IOException {
boolean isTesting = "1".equals(getenv("SPARK_TESTING"));
if (prependClasses || isTesting) {
String scala = getScalaVersion();
// All projects _except_ assembly and the external/ projects
// All projects except assemblies:
List<String> projects = Arrays.asList(
"common/network-common",
"common/network-shuffle",
Expand All @@ -155,6 +155,15 @@ List<String> buildClassPath(String appClassPath) throws IOException {
"core",
"docker-integration-tests",
"examples",
"external/akka",
"external/flume",
"external/flume-sink",
"external/kafka",
"external/mqtt",
"external/twitter",
"external/zeromq",
"extras/kinesis-asl",
Copy link
Member

@srowen srowen Mar 10, 2016

Choose a reason for hiding this comment

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

Same here

"extras/spark-ganglia-lgpl",
"graphx",
"launcher",
"mllib",
Expand All @@ -166,7 +175,8 @@ List<String> buildClassPath(String appClassPath) throws IOException {
"streaming",
"tags",
"tools",
"unsafe"
"unsafe",
"yarn"
);
if (prependClasses) {
if (!isTesting) {
Expand Down
24 changes: 13 additions & 11 deletions tools/src/main/scala/org/apache/spark/tools/GenerateMIMAIgnore.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ object GenerateMIMAIgnore {
private val mirror = runtimeMirror(classLoader)

private def isDeveloperApi(sym: unv.Symbol) = sym.annotations.exists {
_.tpe =:= mirror.staticClass("org.apache.spark.annotation.DeveloperApi").toType
_.tree.tpe =:= mirror.staticClass("org.apache.spark.annotation.DeveloperApi").toType
}

private def isExperimental(sym: unv.Symbol) = sym.annotations.exists {
_.tpe =:= mirror.staticClass("org.apache.spark.annotation.Experimental").toType
_.tree.tpe =:= mirror.staticClass("org.apache.spark.annotation.Experimental").toType
}


Expand All @@ -66,7 +66,7 @@ object GenerateMIMAIgnore {
val ignoredClasses = mutable.HashSet[String]()
val ignoredMembers = mutable.HashSet[String]()

for (className <- classes) {
for (className <- classes.toSeq.sorted) {
try {
val classSymbol = mirror.classSymbol(Class.forName(className, false, classLoader))
val moduleSymbol = mirror.staticModule(className)
Expand All @@ -90,9 +90,8 @@ object GenerateMIMAIgnore {
if (directlyPrivateSpark || indirectlyPrivateSpark || developerApi || experimental) {
ignoredClasses += className
}
// check if this class has package-private/annotated members.
ignoredMembers ++= getAnnotatedOrPackagePrivateMembers(classSymbol)

ignoredMembers ++= getInnerFunctions(className)
} catch {
// scalastyle:off println
case _: Throwable => println("Error instrumenting class:" + className)
Expand All @@ -104,16 +103,19 @@ object GenerateMIMAIgnore {

/** Scala reflection does not let us see inner function even if they are upgraded
* to public for some reason. So had to resort to java reflection to get all inner
* functions with $$ in there name.
* functions with $$ in their name.
*/
def getInnerFunctions(classSymbol: unv.ClassSymbol): Seq[String] = {
def getInnerFunctions(className: String): Seq[String] = {
try {
Class.forName(classSymbol.fullName, false, classLoader).getMethods.map(_.getName)
.filter(_.contains("$$")).map(classSymbol.fullName + "." + _)
Class.forName(className, false, classLoader)
.getMethods
.map(_.getName)
Copy link
Contributor Author

@JoshRosen JoshRosen Mar 11, 2016

Choose a reason for hiding this comment

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

In addition to the reformatting here, I also changed classSymbol.fullName to className. The problem is that classSymbol.fullName uses underscores instead of dollar signs when denoting inner classes and had some other weirdness, so the old name was causing ClassNotFoundException when it was passed to Class.forName. This caused the flood of `"Unable to detect inner functions" warnings in the MiMa logs, e.g.

[WARN] Unable to detect inner functions for class:org.apache.spark.sql.catalyst.parser.SparkSqlParser_FromClauseParser.partitionTableFunctionSource_return
[WARN] Unable to detect inner functions for class:org.apache.spark.sql.api.r.SQLUtils.RegexContext
[WARN] Unable to detect inner functions for class:org.apache.spark.sql.catalyst.parser.SparkSqlParser_ExpressionParser.$DFA21

.filter(_.contains("$$"))
.map(className + "." + _)
} catch {
case t: Throwable =>
// scalastyle:off println
println("[WARN] Unable to detect inner functions for class:" + classSymbol.fullName)
println("[WARN] Unable to detect inner functions for class:" + className)
// scalastyle:on println
Seq.empty[String]
}
Expand All @@ -124,7 +126,7 @@ object GenerateMIMAIgnore {
x.fullName.startsWith("java") || x.fullName.startsWith("scala")
).filter(x =>
isPackagePrivate(x) || isDeveloperApi(x) || isExperimental(x)
).map(_.fullName) ++ getInnerFunctions(classSymbol)
).map(_.fullName)
}

def main(args: Array[String]) {
Expand Down