You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by xueyumusic <gi...@git.apache.org> on 2018/06/15 05:42:24 UTC
[GitHub] spark pull request #21575: spark.storage.blockManagerSlaveTimeoutMs default ...
GitHub user xueyumusic opened a pull request:
https://github.com/apache/spark/pull/21575
spark.storage.blockManagerSlaveTimeoutMs default config
## What changes were proposed in this pull request?
This PR use spark.network.timeout in place of spark.storage.blockManagerSlaveTimeoutMs when it is not configured, as configuration doc said
## How was this patch tested?
manual test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xueyumusic/spark slaveTimeOutConfig
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21575.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 #21575
----
commit f5943410efd2f8f0cc82493eee5c5a4c30f7ebe3
Author: xueyu <27...@...>
Date: 2018-06-15T05:32:33Z
blockManagerSlaveTimeoutMs default config
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r196924766
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -75,16 +76,18 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
+ sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs",
--- End diff --
Looks like we never use `slaveTimeoutMs`. Let's delete this `val` and just assign this value to `executorTimeoutMs`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21575
**[Test build #92160 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92160/testReport)** for PR 21575 at commit [`025bfb3`](https://github.com/apache/spark/commit/025bfb32648a4cbd2e98d5a7948449b063df28f4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r196428732
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -75,16 +76,18 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
+ sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs",
--- End diff --
The val `slaveTimeoutMs` is only used in https://github.com/apache/spark/blob/c6ff59a230758b409fa9cc548b7d283eeb7ebe5d/core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala#L80, so I don't think this change shall fix anything.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r196429048
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -75,16 +76,18 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
+ sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs",
--- End diff --
cc @zsxwing to confirm.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r198579275
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -74,17 +75,17 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
- private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
private val executorTimeoutMs =
- sc.conf.getTimeAsSeconds("spark.network.timeout", s"${slaveTimeoutMs}ms") * 1000
+ sc.conf.getTimeAsSeconds("spark.network.timeout",
+ s"${sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs",
+ "120s")}ms").seconds.toMillis
// "spark.network.timeoutInterval" uses "seconds", while
// "spark.storage.blockManagerTimeoutIntervalMs" uses "milliseconds"
- private val timeoutIntervalMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerTimeoutIntervalMs", "60s")
private val checkTimeoutIntervalMs =
- sc.conf.getTimeAsSeconds("spark.network.timeoutInterval", s"${timeoutIntervalMs}ms") * 1000
+ sc.conf.getTimeAsSeconds("spark.network.timeoutInterval",
--- End diff --
please revert this since it's unrelated.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: spark.storage.blockManagerSlaveTimeoutMs default config
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21575
Have you checked if the other parameters (spark.shuffle.io.connectionTimeout, spark.rpc.askTimeout or spark.rpc.lookupTimeout) could have the same issue?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r198576712
--- Diff: core/src/test/scala/org/apache/spark/SparkConfSuite.scala ---
@@ -371,6 +371,23 @@ class SparkConfSuite extends SparkFunSuite with LocalSparkContext with ResetSyst
assert(thrown.getMessage.contains(key))
}
}
+
+ test("SPARK-24566") {
--- End diff --
This test is useless. It tests copy-pasted codes and if someone changes the original codes, this will still pass. I prefer to not add this one.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/21575
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by xueyumusic <gi...@git.apache.org>.
Github user xueyumusic commented on the issue:
https://github.com/apache/spark/pull/21575
It seems that "spark.core.connection.ack.wait.timeout" and "spark.shuffle.io.connectionTimeout" are used only in tests which might be legacy and do not have an impact on normal code, and "spark.rpc.lookupTimeout" don't have the same issue.
The only one for "spark.rpc.askTimeout" which I am not sure whether it is an issue is https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/deploy/Client.scala#L229. I am not sure whether it is a special case that force this config 10s when not configured
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by xueyumusic <gi...@git.apache.org>.
Github user xueyumusic commented on the issue:
https://github.com/apache/spark/pull/21575
I added the tests, thanks @maropu
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/21575
Jenkins, retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21575
**[Test build #92479 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92479/testReport)** for PR 21575 at commit [`536025d`](https://github.com/apache/spark/commit/536025d98cf5f1a11415c67b385e689ec6074699).
* 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 #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92004/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: spark.storage.blockManagerSlaveTimeoutMs default config
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21575
**[Test build #92479 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92479/testReport)** for PR 21575 at commit [`536025d`](https://github.com/apache/spark/commit/536025d98cf5f1a11415c67b385e689ec6074699).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21575
**[Test build #92004 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92004/testReport)** for PR 21575 at commit [`9673613`](https://github.com/apache/spark/commit/967361302682ad86437261e704c1b14b933d3a86).
* 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 #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
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 pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by xueyumusic <gi...@git.apache.org>.
Github user xueyumusic commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r197028516
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -75,16 +76,18 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
+ sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs",
--- End diff --
I have removed temp val `slaveTimeout`, also `timeoutIntervalMs` is the same case, so removed too, thanks @zsxwing @jiangxb1987
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92479/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21575
btw, better to add tests and can you do?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92160/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21575
**[Test build #92160 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92160/testReport)** for PR 21575 at commit [`025bfb3`](https://github.com/apache/spark/commit/025bfb32648a4cbd2e98d5a7948449b063df28f4).
* This patch **fails due to an unknown error code, -9**.
* 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 pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r198578198
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -74,17 +75,17 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
- private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
private val executorTimeoutMs =
- sc.conf.getTimeAsSeconds("spark.network.timeout", s"${slaveTimeoutMs}ms") * 1000
+ sc.conf.getTimeAsSeconds("spark.network.timeout",
--- End diff --
I meant something like this to match the docs:
```
private val executorTimeoutMs =
sc.conf.getTimeAsMs(
"spark.storage.blockManagerSlaveTimeoutMs",
s"${sc.conf.getTimeAsSeconds("spark.network.timeout", "120s")}s")
```
Could you also change https://github.com/apache/spark/blob/53c06ddabbdf689f8823807445849ad63173676f/resource-managers/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackend.scala#L637 to use the above pattern?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21575
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21575
**[Test build #92004 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92004/testReport)** for PR 21575 at commit [`9673613`](https://github.com/apache/spark/commit/967361302682ad86437261e704c1b14b933d3a86).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21575
cc: @jiangxb1987
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: spark.storage.blockManagerSlaveTimeoutMs default config
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by xueyumusic <gi...@git.apache.org>.
Github user xueyumusic commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r199143954
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -74,17 +75,17 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
- private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
private val executorTimeoutMs =
- sc.conf.getTimeAsSeconds("spark.network.timeout", s"${slaveTimeoutMs}ms") * 1000
+ sc.conf.getTimeAsSeconds("spark.network.timeout",
--- End diff --
updated, please have a review, thank you @zsxwing @jiangxb1987
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
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 #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:
https://github.com/apache/spark/pull/21575
LGTM. Thanks! Merging to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21575
**[Test build #91985 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91985/testReport)** for PR 21575 at commit [`9673613`](https://github.com/apache/spark/commit/967361302682ad86437261e704c1b14b933d3a86).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by xueyumusic <gi...@git.apache.org>.
Github user xueyumusic commented on the issue:
https://github.com/apache/spark/pull/21575
I have made the modification, @maropu please review the code, thank you
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by xueyumusic <gi...@git.apache.org>.
Github user xueyumusic commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r196635402
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -75,16 +76,18 @@ private[spark] class HeartbeatReceiver(sc: SparkContext, clock: Clock)
// "spark.network.timeout" uses "seconds", while `spark.storage.blockManagerSlaveTimeoutMs` uses
// "milliseconds"
private val slaveTimeoutMs =
- sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs", "120s")
+ sc.conf.getTimeAsMs("spark.storage.blockManagerSlaveTimeoutMs",
--- End diff --
I look at this carefully, I think your are right, thanks @jiangxb1987 . One case that is not relevant with this PR is like this: set spark.storage.blockManagerSlaveTimeoutMs=900ms and not configure spark.network.timeout, then `executorTimeoutMs ` will be 0 since getTimeAsSeconds loos precision for ms. This config maybe not reasonable. If need fix how about add ensuring > 0 or make executorTimeoutMs's min value as 1, @jiangxb1987 @zsxwing
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21575: [SPARK-24566][CORE] spark.storage.blockManagerSlaveTimeo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21575
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21575: [SPARK-24566][CORE] spark.storage.blockManagerSla...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21575#discussion_r198579348
--- Diff: core/src/main/scala/org/apache/spark/HeartbeatReceiver.scala ---
@@ -21,6 +21,7 @@ import java.util.concurrent.{ScheduledFuture, TimeUnit}
import scala.collection.mutable
import scala.concurrent.Future
+import scala.concurrent.duration._
--- End diff --
this will be unused after you address my comments
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org