You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sandeep-katta <gi...@git.apache.org> on 2018/06/14 10:35:48 UTC
[GitHub] spark pull request #21565: wrong Idle Timeout value is used in case of the c...
GitHub user sandeep-katta opened a pull request:
https://github.com/apache/spark/pull/21565
wrong Idle Timeout value is used in case of the cacheBlock.
It is corrected as per the configuration.
## What changes were proposed in this pull request?
IdleTimeout info used to print in the logs is taken based on the cacheBlock. If it is cacheBlock then cachedExecutorIdleTimeoutS is considered else executorIdleTimeoutS
## How was this patch tested?
Manual Test
spark-sql> cache table sample;
2018-05-15 14:44:02 INFO DAGScheduler:54 - Submitting 3 missing tasks from ShuffleMapStage 0 (MapPartitionsRDD[8] at processCmd at CliDriver.java:376) (first 15 tasks are for partitions Vector(0, 1, 2))
2018-05-15 14:44:02 INFO YarnScheduler:54 - Adding task set 0.0 with 3 tasks
2018-05-15 14:44:03 INFO ExecutorAllocationManager:54 - Requesting 1 new executor because tasks are backlogged (new desired total will be 1)
...
...
2018-05-15 14:46:10 INFO YarnClientSchedulerBackend:54 - Actual list of executor(s) to be killed is 1
2018-05-15 14:46:10 INFO **ExecutorAllocationManager:54 - Removing executor 1 because it has been idle for 120 seconds (new desired total will be 0)**
2018-05-15 14:46:11 INFO YarnSchedulerBackend$YarnDriverEndpoint:54 - Disabling executor 1.
2018-05-15 14:46:11 INFO DAGScheduler:54 - Executor lost: 1 (epoch 1)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sandeep-katta/spark loginfoBug
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21565.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 #21565
----
commit 30fcef650ee2bd2873bf402448652acba055f989
Author: sandeep-katta <sa...@...>
Date: 2018-06-14T09:56:59Z
wrong Idle Timeout value is used in case of the cacheBlock.
It is corrected as per the configuration.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92179/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: wrong Idle Timeout value is used in case of the cacheBlo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21565
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21565
have you addressed all the comments?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][SPARK-Core]wrong Idle Timeout value is use...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21565
nit: could you please replace `[Spark-Core]` with `[Core]`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21565
have you addressed https://github.com/apache/spark/pull/21565#discussion_r195894664 ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21565
**[Test build #92932 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92932/testReport)** for PR 21565 at commit [`a67107a`](https://github.com/apache/spark/commit/a67107a85a9b7e352f18bea94cee0f84e7940e97).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/21565
@cloud-fan Build is passed.Can you push this PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r195989897
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
--- End diff --
@maropu can you review and merge the PR if the changes are fine
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21565
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r198447523
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
+ val idleTimeout = if (blockManagerMaster.hasCachedBlocks(removedExecutorId)) {
--- End diff --
blockManagerMaster already provides API to check it is cached block or not,so I feel it will be overhead to maintain another HashMap
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21565
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r195894845
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
--- End diff --
yes, if it is cached block then IDLE time out taken from the spark.dynamicAllocation.cachedExecutorIdleTimeout
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92178/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r195894664
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
--- End diff --
`// If it is a cached block, it uses cachedExecutorIdleTimeoutS for timeout`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by Ngone51 <gi...@git.apache.org>.
Github user Ngone51 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r200019612
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
+ val idleTimeout = if (blockManagerMaster.hasCachedBlocks(removedExecutorId)) {
--- End diff --
We do not maintain another HashMap, but alter its original structure. In this way, we do not need to issue extra rpc calls to `BlockManagerMaster` here. As you mentioned 'API', this thing happens after a rpc call happened.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/92932/
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r195894599
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
+ val idleTimeout = if (SparkEnv.get.blockManager.master.hasCachedBlocks(removedExecutorId)) {
--- End diff --
`blockManagerMaster.hasCachedBlocks(removedExecutorId)`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/21565
@cloud-fan can you please review this small piece of code and merge this PR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21565
**[Test build #92179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92179/testReport)** for PR 21565 at commit [`a5708cc`](https://github.com/apache/spark/commit/a5708cc59786edeae792e926821d8c26b01ee39d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: wrong Idle Timeout value is used in case of the cacheBlo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21565
**[Test build #92178 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92178/testReport)** for PR 21565 at commit [`a5708cc`](https://github.com/apache/spark/commit/a5708cc59786edeae792e926821d8c26b01ee39d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: wrong Idle Timeout value is used in case of the cacheBlo...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by Ngone51 <gi...@git.apache.org>.
Github user Ngone51 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r195927590
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
+ val idleTimeout = if (blockManagerMaster.hasCachedBlocks(removedExecutorId)) {
--- End diff --
How about changing `removeTimes` to `HashMap[String, (Long, Boolean)]` (and the `Boolean` field indicates whether it is for cachedExecutor idle timeout or not) ? Thus, we do not need to ask `blockManagerMaster` again.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21565
**[Test build #92178 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92178/testReport)** for PR 21565 at commit [`a5708cc`](https://github.com/apache/spark/commit/a5708cc59786edeae792e926821d8c26b01ee39d).
* 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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21565
**[Test build #92932 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92932/testReport)** for PR 21565 at commit [`a67107a`](https://github.com/apache/spark/commit/a67107a85a9b7e352f18bea94cee0f84e7940e97).
* 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 pull request #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r195895035
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
+ val idleTimeout = if (SparkEnv.get.blockManager.master.hasCachedBlocks(removedExecutorId)) {
--- End diff --
I have updated the code as per comments
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21565
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21565
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 #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/21565
@cloud-fan did this merge?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on the issue:
https://github.com/apache/spark/pull/21565
yes all the review comments are addressed
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r197237620
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
--- End diff --
can you refine the wording?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21565
**[Test build #92179 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92179/testReport)** for PR 21565 at commit [`a5708cc`](https://github.com/apache/spark/commit/a5708cc59786edeae792e926821d8c26b01ee39d).
* 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 pull request #21565: [SPARK-24558][Core]wrong Idle Timeout value is us...
Posted by sandeep-katta <gi...@git.apache.org>.
Github user sandeep-katta commented on a diff in the pull request:
https://github.com/apache/spark/pull/21565#discussion_r195894823
--- Diff: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala ---
@@ -488,9 +488,16 @@ private[spark] class ExecutorAllocationManager(
newExecutorTotal = numExistingExecutors
if (testing || executorsRemoved.nonEmpty) {
executorsRemoved.foreach { removedExecutorId =>
+ // If it is cachedBlcok timeout is configured using
+ // spark.dynamicAllocation.cachedExecutorIdleTimeout
+ val idleTimeout = if (SparkEnv.get.blockManager.master.hasCachedBlocks(removedExecutorId)) {
--- End diff --
yes blockManagerMaster is referring to same SparkEnv.get.blockManager.master. Will update the code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21565: [SPARK-24558][Core]wrong Idle Timeout value is used in c...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21565
seems something was wrong with my merge script... It's merged now, @srowen thanks for reminding!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org