[SPARK-12413] Fix Mesos ZK persistence #10366

mgummelt · 2015-12-17T22:38:32Z

I believe this fixes SPARK-12413. I'm currently running an integration test to verify.

andrewor14 · 2015-12-17T22:43:18Z

add to whitelist

andrewor14 · 2015-12-17T22:44:19Z

core/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala

+    //
+    // Due to https://issues.scala-lang.org/browse/SI-6654, `filterKeys` returns an
+    // unserializable object,so we must call `.map(identity)` on the result.
+    val environmentVariables = request.environmentVariables.filterKeys(!_.equals("SPARK_HOME")).map(identity)


this line will fail style tests because it's too long. Need to break it into 2

fixed. thanks.

SparkQA · 2015-12-17T22:53:38Z

Test build #47959 has finished for PR 10366 at commit bbfbbdf.

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

andrewor14 · 2015-12-17T22:54:31Z

I was able to verify SI-6654 in the REPL and I believe this patch is the correct fix:

val map = Map[String, String]("a" -> "b", "c" -> "d")
val filteredMap = map.filterKeys(_ != "a")
val identitiedMap = filteredMap.map(identity)

// OK
(new ObjectOutputStream(new ByteArrayOutputStream)).writeObject(map)

// NotSerializableException: scala.collection.immutable.MapLike$$anon$1
(new ObjectOutputStream(new ByteArrayOutputStream)).writeObject(filteredMap)

// OK
(new ObjectOutputStream(new ByteArrayOutputStream)).writeObject(identitiedMap)

andrewor14 · 2015-12-17T22:56:36Z

LGTM, retest this please

andrewor14 · 2015-12-17T22:56:54Z

@mgummelt is this still WIP? If not can you remove that from the title? Also, any updates from your integration tests?

mgummelt · 2015-12-17T22:57:48Z

I included [WIP] to indicate that my integration test hasn't finished yet (still pending). Code should be frozen though. Would you like me to remove [WIP]?

andrewor14 · 2015-12-17T22:58:08Z

Yes

andrewor14 · 2015-12-17T23:02:44Z

core/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala

    val environmentVariables = request.environmentVariables.filterKeys(!_.equals("SPARK_HOME"))
+      .map(identity)


Actually, there's a slightly nicer way...

// Do not use `filterKeys` here to avoid SI-6654, which breaks ZK persistence val environmentVariables = request.environmentVariables.filter { case (k, _) => k != "SPARK_HOME" }

so we don't have to do the weird filterKeys(...).map(identity) thing

Yes, that's better. Fixed.

mgummelt · 2015-12-17T23:12:31Z

Fixed

SparkQA · 2015-12-17T23:19:03Z

Test build #2226 has started for PR 10366 at commit d70bcee.

SparkQA · 2015-12-17T23:19:11Z

Test build #2225 has started for PR 10366 at commit d70bcee.

mgummelt · 2015-12-17T23:34:30Z

Test failure looks like some sort of timeout? https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47962/console

andrewor14 · 2015-12-17T23:57:49Z

It's OK, we have 2 builds that are actually running. Hopefully one of them will pass.

andrewor14 · 2015-12-18T02:15:09Z

retest this please. I'm not sure why it's timing out on tests. AFAIK this file isn't even used in tests.

SparkQA · 2015-12-18T02:18:27Z

Test build #2229 has started for PR 10366 at commit d70bcee.

SparkQA · 2015-12-18T03:11:37Z

Test build #47960 has finished for PR 10366 at commit 05c4da3.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds no public classes.

nraychaudhuri · 2015-12-18T03:24:51Z

retest this please

SparkQA · 2015-12-18T04:37:02Z

Test build #47974 has finished for PR 10366 at commit d70bcee.

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

SparkQA · 2015-12-18T04:45:58Z

Test build #2228 has finished for PR 10366 at commit d70bcee.

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

SparkQA · 2015-12-18T04:52:41Z

Test build #2230 has finished for PR 10366 at commit d70bcee.

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

keithchambers · 2015-12-18T05:53:58Z

Will this make 1.6.0?

dragos · 2015-12-18T09:31:50Z

LGTM. Thanks @mgummelt for catching and fixing this!

tnachen · 2015-12-18T10:32:00Z

LGTM as well.

sarutak · 2015-12-18T11:16:58Z

LGTM. Merging into master and branch-1.6. Thanks @mgummelt !

I believe this fixes SPARK-12413. I'm currently running an integration test to verify. Author: Michael Gummelt <mgummelt@mesosphere.io> Closes #10366 from mgummelt/fix-zk-mesos. (cherry picked from commit 2bebaa3) Signed-off-by: Kousuke Saruta <sarutak@oss.nttdata.co.jp>

andrewor14 · 2015-12-19T00:24:49Z

Note: I'm reverting this patch in master only since #10329, the better alternative, is merged there.
This patch continues to exist in branch-1.6.

keithchambers · 2015-12-19T16:33:50Z

@andrewor14 makes sense. Thank you!

tedyu · 2015-12-25T18:16:24Z

core/src/main/scala/org/apache/spark/deploy/rest/mesos/MesosRestServer.scala

@@ -99,7 +99,11 @@ private[mesos] class MesosSubmitRequestServlet(
    // cause spark-submit script to look for files in SPARK_HOME instead.
    // We only need the ability to specify where to find spark-submit script
    // which user can user spark.executor.home or spark.home configurations.
-    val environmentVariables = request.environmentVariables.filterKeys(!_.equals("SPARK_HOME"))
+    //
+    // Do not use `filterKeys` here to avoid SI-6654, which breaks ZK persistence


Should a scalastyle rule be added which prevents filterKeys to be used in future PRs ?

[SPARK-12413] Fix Mesos ZK persistence

bbfbbdf

andrewor14 reviewed Dec 17, 2015
View reviewed changes

comment spacing and line length fix

05c4da3

mgummelt changed the title ~~[WIP] [SPARK-12413] Fix Mesos ZK persistence~~ [SPARK-12413] Fix Mesos ZK persistence Dec 17, 2015

andrewor14 reviewed Dec 17, 2015
View reviewed changes

use filter instead of filterKeys

d70bcee

mgummelt closed this Dec 17, 2015

mgummelt reopened this Dec 17, 2015

asfgit closed this in 2bebaa3 Dec 18, 2015

This was referenced Dec 19, 2015

[SPARK-12345][Mesos] Properly filter out SPARK_HOME in the Mesos REST server #10359

Closed

[SPARK-12345] [CORE] Do not send SPARK_HOME through Spark submit REST interface #10329

Closed

tedyu reviewed Dec 25, 2015
View reviewed changes

mgummelt deleted the fix-zk-mesos branch August 2, 2016 20:58

		val environmentVariables = request.environmentVariables.filterKeys(!_.equals("SPARK_HOME"))
		.map(identity)

[SPARK-12413] Fix Mesos ZK persistence #10366

[SPARK-12413] Fix Mesos ZK persistence #10366

Uh oh!

Conversation

mgummelt commented Dec 17, 2015

Uh oh!

andrewor14 commented Dec 17, 2015

Uh oh!

andrewor14 Dec 17, 2015

Choose a reason for hiding this comment

Uh oh!

mgummelt Dec 17, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 17, 2015

Uh oh!

andrewor14 commented Dec 17, 2015

Uh oh!

andrewor14 commented Dec 17, 2015

Uh oh!

andrewor14 commented Dec 17, 2015

Uh oh!

mgummelt commented Dec 17, 2015

Uh oh!

andrewor14 commented Dec 17, 2015

Uh oh!

andrewor14 Dec 17, 2015

Choose a reason for hiding this comment

Uh oh!

mgummelt Dec 17, 2015

Choose a reason for hiding this comment

Uh oh!

mgummelt commented Dec 17, 2015

Uh oh!

SparkQA commented Dec 17, 2015

Uh oh!

SparkQA commented Dec 17, 2015

Uh oh!

mgummelt commented Dec 17, 2015

Uh oh!

andrewor14 commented Dec 17, 2015

Uh oh!

andrewor14 commented Dec 18, 2015

Uh oh!

SparkQA commented Dec 18, 2015

Uh oh!

SparkQA commented Dec 18, 2015

Uh oh!

nraychaudhuri commented Dec 18, 2015

Uh oh!

SparkQA commented Dec 18, 2015

Uh oh!

SparkQA commented Dec 18, 2015

Uh oh!

SparkQA commented Dec 18, 2015

Uh oh!

keithchambers commented Dec 18, 2015

Uh oh!

dragos commented Dec 18, 2015

Uh oh!

tnachen commented Dec 18, 2015

Uh oh!

sarutak commented Dec 18, 2015

Uh oh!

andrewor14 commented Dec 19, 2015

Uh oh!

keithchambers commented Dec 19, 2015

Uh oh!

tedyu Dec 25, 2015

Choose a reason for hiding this comment

Uh oh!

Uh oh!