You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by tgravescs <gi...@git.apache.org> on 2017/10/06 18:58:26 UTC
[GitHub] spark pull request #19450: [SPARK-22218] spark shuffle services fails to upd...
GitHub user tgravescs opened a pull request:
https://github.com/apache/spark/pull/19450
[SPARK-22218] spark shuffle services fails to update secret on app re-attempts
This patch fixes application re-attempts when running spark on yarn using the external shuffle service with security on. Currently executors will fail to launch on any application re-attempt when launched on a nodemanager that had an executor from the first attempt. The reason for this is because we aren't updating the secret key after the first application attempt. The fix here is to just remove the containskey check to see if it already exists. In this way, we always add it and make sure its the most recent secret. Similarly remove the check for containsKey on the remove since its just adding extra check that isn't really needed.
Note this worked before spark 2.2 because the check used to be contains (which was looking for the value) rather then containsKey, so that never matched and it was just always adding the new secret.
Patch was tested on a 10 node cluster as well as added the unit test.
The test ran was a wordcount where the output directory already existed. With the bug present the application attempt failed with max number of executor Failures which were all saslExceptions. With the fix present the application re-attempts fail with directory already exists or when you remove the directory between attempts the re-attemps succeed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tgravescs/spark SPARK-22218
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19450.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #19450
----
commit aa9a2c2705cd59f9e1caa7e78ad7eafad3ff2789
Author: Thomas Graves <tg...@unharmedunarmed.corp.ne1.yahoo.com>
Date: 2017-10-06T18:45:13Z
[SPARK-22218] spark shuffle services fails to update secret on application re-attempts
commit 5a9ef1396a28b535042e336a2f43d80104ce95e5
Author: Thomas Graves <tg...@unharmedunarmed.corp.ne1.yahoo.com>
Date: 2017-10-06T18:55:43Z
minor updates
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19450: [SPARK-22218] spark shuffle services fails to update sec...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19450
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82521/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19450: [SPARK-22218] spark shuffle services fails to update sec...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19450
**[Test build #82521 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82521/testReport)** for PR 19450 at commit [`5a9ef13`](https://github.com/apache/spark/commit/5a9ef1396a28b535042e336a2f43d80104ce95e5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19450: [SPARK-22218] spark shuffle services fails to update sec...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/19450
Merging to master / 2.2.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19450: [SPARK-22218] spark shuffle services fails to update sec...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the issue:
https://github.com/apache/spark/pull/19450
thanks @vanzin, sorry I had made the test changes but hadn't pushed them.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19450: [SPARK-22218] spark shuffle services fails to update sec...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19450
**[Test build #82521 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82521/testReport)** for PR 19450 at commit [`5a9ef13`](https://github.com/apache/spark/commit/5a9ef1396a28b535042e336a2f43d80104ce95e5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19450: [SPARK-22218] spark shuffle services fails to upd...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/19450#discussion_r143294931
--- Diff: common/network-shuffle/src/test/java/org/apache/spark/network/sasl/ShuffleSecretManagerSuite.java ---
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.network.sasl;
+
+import java.nio.ByteBuffer;
+
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+public class ShuffleSecretManagerSuite {
+ static String app1 = "app1";
--- End diff --
`private final` for all of these?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19450: [SPARK-22218] spark shuffle services fails to upd...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19450
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19450: [SPARK-22218] spark shuffle services fails to update sec...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19450
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19450: [SPARK-22218] spark shuffle services fails to update sec...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the issue:
https://github.com/apache/spark/pull/19450
ping @vanzin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org