[SPARK-12413] Fix Mesos ZK persistence

[mgummelt]

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

[andrewor14]

add to whitelist

[andrewor14]

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

[mgummelt]

fixed. thanks.

[SparkQA]

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]

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]

LGTM, retest this please

[andrewor14]

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

[mgummelt]

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]

Yes

[andrewor14]

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

[mgummelt]

Yes, that's better. Fixed.

[mgummelt]

Fixed

[SparkQA]

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

[SparkQA]

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

[mgummelt]

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

[andrewor14]

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

[andrewor14]

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

[SparkQA]

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

[SparkQA]

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]

retest this please

[SparkQA]

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]

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]

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]

Will this make 1.6.0?

[dragos]

LGTM. Thanks @mgummelt for catching and fixing this!

[tnachen]

LGTM as well.

[sarutak]

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

[andrewor14]

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]

@andrewor14 makes sense. Thank you!

[tedyu]

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