[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-168112823 I'm going to close this pull request. If this is still relevant and you are interested in pushing it forward, please open a new pull request. Thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/6263 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user lianhuiwang commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-160853790 @archit279thakur @CodingCat @squito I created new PR #10058. Can you take a look at it?Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-159640761 In the current version of patch, we use expiration time to prevent too many dead executors from appearing on the UI. It brings inconvenient overhead which makes the UI component to have a dependency on Guava...Additionally, there are cases that the executors are failed and restarted time and time again within a very short period (I met this in some of my applications when I introduced some bug, cannot remember what exactly happened) I'm considering that we might be able to just cap the maximum number of rows in the table, like what we do in many other places (master/worker UI, etc.). Event we stick to the expiration time, TimeStampedHashMap might be a cleaner solution? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-159642220 @squito Regarding the page structure, do not trust my sense of aesthetic, :-) Personally, I prefer to separate the page into two sections, one for alive executors, one for dead ones, or you provide the capability to the user to sort the entries with status --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user CodingCat commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r45875872
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -28,15 +35,39 @@ import org.apache.spark.scheduler._
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+@DeveloperApi
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
--- End diff --
do we really need to have this class? how about just exposing this string
to the end user?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-159635180 @archit279thakur , would you mind just uploading some screenshot, so that we have more sense on the current page structure? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on the pull request:
https://github.com/apache/spark/pull/6263#issuecomment-159409155
Hi @archit279thakur,
good questions about what to do with the time it was killed. The reason I
wanted it included is so the user could put it together with the timeline, to
see what stages were running when the executor was killed, to help them debug
*why* the executor was removed. I dont' have strong opinions about where it
should go in the UI -- I'm willing to believe that it will just lead to too
much clutter. @CodingCat , any thoughts? But in either case, it would still
be nice to have in the json endpoint.
btw, the mima failure is from this:
```
[error] * method
this(java.lang.String,java.lang.String,Int,Long,Long,Int,Int,Int,Int,Long,Long,Long,Long,Long,scala.collection.Map)Unit
in class org.apache.spark.status.api.v1.ExecutorSummary does not have a
correspondent in new version
[error]filter with:
ProblemFilters.exclude[MissingMethodProblem]("org.apache.spark.status.api.v1.ExecutorSummary.this")
```
you can add that line to `project/MimaExcludes.scala`, that constructor is
private so this a false positive.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158377704 **[Test build #46417 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46417/consoleFull)** for PR 6263 at commit [`33fc892`](https://github.com/apache/spark/commit/33fc892c84a0a0ef7375c26eba1e581ffc108a2b). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158385311 **[Test build #46418 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46418/consoleFull)** for PR 6263 at commit [`2cf4f71`](https://github.com/apache/spark/commit/2cf4f718d7a72fbc5fb663d5576bb7de88bfdeff). * This patch **fails to build**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158385324 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46418/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158385319 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158425982 **[Test build #46422 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46422/consoleFull)** for PR 6263 at commit [`e1577dc`](https://github.com/apache/spark/commit/e1577dc1b364aa3a4e923ebe3ddc0e82d270cef5). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158425985 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158425439 **[Test build #46422 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46422/consoleFull)** for PR 6263 at commit [`e1577dc`](https://github.com/apache/spark/commit/e1577dc1b364aa3a4e923ebe3ddc0e82d270cef5). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158425988 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46422/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158357514 **[Test build #46414 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46414/consoleFull)** for PR 6263 at commit [`3e23321`](https://github.com/apache/spark/commit/3e233216df375345ad7e998be9567f3acc903110). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158366439 @squito In reply to: *@archit279thakur can you also bring this up to date with master, and include before & after screenshots? I'd like for this to also update the json endpoints. Finally, I think that as long we're storing removed executors, we should store the time they were removed.* Two things: 1. For that, I'll have to add a new column in the execTable on the UI. Should it be lastStatusChangedTime (with aliveTime or killedTime, depending on the status) or KilledTime (with blank values for the Alive executors) ? 2. This value would always be greater than the value currentTime - spark.ui.timeToExpireKilledExecutor. I am not really sure, we provide any useful insights by showing the time at which executor died. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158357990 **[Test build #46414 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46414/consoleFull)** for PR 6263 at commit [`3e23321`](https://github.com/apache/spark/commit/3e233216df375345ad7e998be9567f3acc903110). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158357994 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46414/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158357992 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158369809 **[Test build #46416 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46416/consoleFull)** for PR 6263 at commit [`b827f8f`](https://github.com/apache/spark/commit/b827f8f0c8f463cc181608e927add71a2cae71b0). * This patch **fails MiMa tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158369893 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46416/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158369890 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158378393 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46417/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158378387 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158378375 **[Test build #46417 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46417/consoleFull)** for PR 6263 at commit [`33fc892`](https://github.com/apache/spark/commit/33fc892c84a0a0ef7375c26eba1e581ffc108a2b). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158382654 **[Test build #46418 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46418/consoleFull)** for PR 6263 at commit [`2cf4f71`](https://github.com/apache/spark/commit/2cf4f718d7a72fbc5fb663d5576bb7de88bfdeff). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158366325 **[Test build #46416 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46416/consoleFull)** for PR 6263 at commit [`b827f8f`](https://github.com/apache/spark/commit/b827f8f0c8f463cc181608e927add71a2cae71b0). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158435009 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46423/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158435008 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158430559 **[Test build #46423 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46423/consoleFull)** for PR 6263 at commit [`826587f`](https://github.com/apache/spark/commit/826587fab9e2976092c0ed54b385d4c8cb596fbf). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-158434952 **[Test build #46423 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46423/consoleFull)** for PR 6263 at commit [`826587f`](https://github.com/apache/spark/commit/826587fab9e2976092c0ed54b385d4c8cb596fbf). * This patch **fails MiMa tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157310649 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46074/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157310644 [Test build #46074 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46074/consoleFull) for PR 6263 at commit [`1a20371`](https://github.com/apache/spark/commit/1a2037102bb0067f145a1401a495d86dc4a136f7). * This patch **fails to build**. * This patch **does not merge cleanly**. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157310646 Build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157310550 [Test build #46074 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46074/consoleFull) for PR 6263 at commit [`1a20371`](https://github.com/apache/spark/commit/1a2037102bb0067f145a1401a495d86dc4a136f7). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44960862
--- Diff: core/src/main/scala/org/apache/spark/ui/exec/ExecutorsPage.scala
---
@@ -25,6 +25,7 @@ import scala.xml.Node
import org.apache.spark.status.api.v1.ExecutorSummary
import org.apache.spark.ui.{ToolTips, UIUtils, WebUIPage}
import org.apache.spark.util.Utils
+import org.apache.spark.storage.StorageStatus
--- End diff --
nit: ordering
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157128907 @archit279thakur yeah don't worry about that test failure ... when you push updates & bring up to date w/ master the tests will re-run in any case --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44961686
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -87,6 +113,8 @@ class StorageStatusListener extends SparkListener {
override def onBlockManagerRemoved(blockManagerRemoved:
SparkListenerBlockManagerRemoved) {
synchronized {
val executorId = blockManagerRemoved.blockManagerId.executorId
+ removedExecutorIdToStorageStatus.put(executorId,
+ executorIdToStorageStatus.get(executorId).get)
--- End diff --
I really think it would be nice to keep the time the executor was removed
as well from `blockManagerRemoved.time` to show in the UI
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44961121
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -28,15 +35,34 @@ import org.apache.spark.scheduler._
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
--- End diff --
`@DeveloperApi`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157298330 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46065/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157298326 [Test build #46065 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46065/consoleFull) for PR 6263 at commit [`a179aa1`](https://github.com/apache/spark/commit/a179aa14849f5c741a06b7cadff1ded72b9ae4ab). * This patch **fails to build**. * This patch **does not merge cleanly**. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157298329 Build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r45029094
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -87,6 +113,8 @@ class StorageStatusListener extends SparkListener {
override def onBlockManagerRemoved(blockManagerRemoved:
SparkListenerBlockManagerRemoved) {
synchronized {
val executorId = blockManagerRemoved.blockManagerId.executorId
+ removedExecutorIdToStorageStatus.put(executorId,
+ executorIdToStorageStatus.get(executorId).get)
--- End diff --
Two things:
1. For that, I'll have to add a new column in the `execTable` on the UI.
Should it be lastStatusChangedTime or KilledTime with blank values for the
Alive executors?
2. This value would always be greater than the value `currentTime -
spark.ui.timeToExpireKilledExecutor`. I am not really sure, we provide any
useful insights by showing the time at which executor died.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-157296312 [Test build #46065 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46065/consoleFull) for PR 6263 at commit [`a179aa1`](https://github.com/apache/spark/commit/a179aa14849f5c741a06b7cadff1ded72b9ae4ab). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44892842
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -28,15 +36,33 @@ import org.apache.spark.scheduler._
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
+}
+
@DeveloperApi
-class StorageStatusListener extends SparkListener {
+class StorageStatusListener private[storage](conf: SparkConf, ticker:
Ticker) {
+ def this(conf: SparkConf) = {
+this(conf, Ticker.systemTicker())
+ }
+
+ import StorageStatusListener._
+
// This maintains only blocks that are cached (i.e. storage level is not
StorageLevel.NONE)
private[storage] val executorIdToStorageStatus = mutable.Map[String,
StorageStatus]()
+ private[storage] val removedExecutorIdToStorageStatus =
CacheBuilder.newBuilder().
+expireAfterWrite(conf.getTimeAsSeconds(TIME_TO_EXPIRE_KILLED_EXECUTOR,
"0"), TimeUnit.SECONDS).
+ticker(ticker).build[String, StorageStatus]()
--- End diff --
I was accessing it in tests, so kept it private[storage]. I can make it
private and addition of new tests, (if require) can make it private[storage] ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-156940725 [Test build #45984 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45984/consoleFull) for PR 6263 at commit [`cef7c8b`](https://github.com/apache/spark/commit/cef7c8b8b82c7cc3bcbf99e66a99b72ca87c35af). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-156941079 Build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-156941081 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/45984/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-156941074 [Test build #45984 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45984/consoleFull) for PR 6263 at commit [`cef7c8b`](https://github.com/apache/spark/commit/cef7c8b8b82c7cc3bcbf99e66a99b72ca87c35af). * This patch **fails to build**. * This patch **does not merge cleanly**. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-156940368 jenkins, please test again. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-156944200 It says: Error: Invalid or corrupt jarfile build/sbt-launch-0.13.7.jar Should I bring it up to date with master first? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44029924
--- Diff:
core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala
---
@@ -150,4 +158,12 @@ class StorageStatusListenerSuite extends FunSuite {
listener.onUnpersistRDD(SparkListenerUnpersistRDD(1))
assert(listener.executorIdToStorageStatus("big").numBlocks === 0)
}
+
+ test("Killed Executor Entry removed after configurable time") {
+val localtestconf = new
SparkConf().set(StorageStatusListener.TIME_TO_EXPIRE_KILLED_EXECUTOR,"5s")
+val listener = new StorageStatusListener(localtestconf)
+listener.removedExecutorIdToStorageStatus.put("1", new
StorageStatus(null, 50))
+Thread.sleep(5500)
--- End diff --
I think you can do something in between -- StorageStatusListener can have a
`private[storage]` constructor which takes the ticker, and the public one just
defaults it to the system ticker. Yes, you would not be testing *exactly* the
same behavior, but it tests the important parts.
Sleeping isn't the worst thing in this case -- often it leads to flaky
tests, though I don't think that would be the case here. Still, 5 seconds is
awfully long for this test when it should take a tiny fraction of that, and it
adds up over all the tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44056583 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -17,26 +17,55 @@ package org.apache.spark.storage +import java.util.concurrent.TimeUnit + +import scala.collection.JavaConversions.collectionAsScalaIterable --- End diff -- Sure. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154179943 [Test build #45141 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45141/consoleFull) for PR 6263 at commit [`1fdffc5`](https://github.com/apache/spark/commit/1fdffc5467c3be0ceb5127c4cfea710e658f15c8). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44055771 --- Diff: core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala --- @@ -17,10 +17,16 @@ package org.apache.spark.storage -import org.scalatest.FunSuite +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicLong + +import org.apache.spark.SparkConf import org.apache.spark.Success import org.apache.spark.executor.TaskMetrics import org.apache.spark.scheduler._ +import org.scalatest.FunSuite + +import com.google.common.base.Ticker --- End diff -- nit: import ordering & grouping --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154167926 @squito Thanks for your comments. Incorporated them all and also gone through the link. Please point out if I missed anything. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154179575 Build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44064388 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -17,26 +17,55 @@ package org.apache.spark.storage +import java.util.concurrent.TimeUnit + +import scala.collection.JavaConversions.collectionAsScalaIterable import scala.collection.mutable +import scala.language.reflectiveCalls --- End diff -- ok, so that is in the StorageStatusListener*Suite*, not StorageStatusListener. You're getting that b/c you're calling `ticker.advance`, though you haven't explicitly defined a class or interface w/ that method, so scala is using some reflection tricks to do it. If you change that test as I suggested, that warning will disappear and you can remove the import --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154180192 [Test build #45141 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45141/consoleFull) for PR 6263 at commit [`1fdffc5`](https://github.com/apache/spark/commit/1fdffc5467c3be0ceb5127c4cfea710e658f15c8). * This patch **fails to build**. * This patch **does not merge cleanly**. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154180197 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45141/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154180196 Build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44055535
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -17,26 +17,55 @@
package org.apache.spark.storage
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConversions.collectionAsScalaIterable
import scala.collection.mutable
+import scala.language.reflectiveCalls
+import org.apache.spark.SparkConf
import org.apache.spark.annotation.DeveloperApi
import org.apache.spark.scheduler._
+import com.google.common.base.Ticker
+import com.google.common.cache.CacheBuilder
+
/**
* :: DeveloperApi ::
* A SparkListener that maintains executor storage status.
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
+}
+
@DeveloperApi
-class StorageStatusListener extends SparkListener {
+class StorageStatusListener(conf: SparkConf) extends SparkListener {
+ var ticker = Ticker.systemTicker()
+
+ private [storage] def this(conf: SparkConf, ticker: Ticker) = {
+this(conf)
+this.ticker = ticker
+ }
--- End diff --
you can avoid the `var` here, though the scala syntax is a little tricky:
```scala
class StorageStatusListener private[storage](conf: SparkConf, ticker:
Ticker) {
def this(conf: SparkConf) = {
this(conf, Ticker.systemTicker())
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44056239 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -17,26 +17,55 @@ package org.apache.spark.storage +import java.util.concurrent.TimeUnit + +import scala.collection.JavaConversions.collectionAsScalaIterable import scala.collection.mutable +import scala.language.reflectiveCalls --- End diff -- Nope. If not imported, compilation gives a warning âreflective access of structural type member method should be enabledâ¦â --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44062329 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -17,26 +17,55 @@ package org.apache.spark.storage +import java.util.concurrent.TimeUnit + +import scala.collection.JavaConversions.collectionAsScalaIterable import scala.collection.mutable +import scala.language.reflectiveCalls --- End diff -- I am getting this warning on the compilation (though there are many already being thrown). [warn] core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala:174: reflective access of structural type member method advance should be enabled [warn] by making the implicit value scala.language.reflectiveCalls visible. [warn] This can be achieved by adding the import clause 'import scala.language.reflectiveCalls' [warn] or by setting the compiler option -language:reflectiveCalls. [warn] See the Scala docs for value scala.language.reflectiveCalls for a discussion [warn] why the feature should be explicitly enabled. [warn] ticker.advance(5, TimeUnit.SECONDS) That's why I imported that. I checked again, I am still getting this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154179771 @archit279thakur can you also bring this up to date with master, and include before & after screenshots? I'd like for this to also update the json endpoints. Finally, I think that as long we're storing removed executors, we should store the time they were removed. @suyanNone can you take another look as well? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154179598 Build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44055655
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -17,26 +17,55 @@
package org.apache.spark.storage
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConversions.collectionAsScalaIterable
import scala.collection.mutable
+import scala.language.reflectiveCalls
+import org.apache.spark.SparkConf
import org.apache.spark.annotation.DeveloperApi
import org.apache.spark.scheduler._
+import com.google.common.base.Ticker
+import com.google.common.cache.CacheBuilder
+
/**
* :: DeveloperApi ::
* A SparkListener that maintains executor storage status.
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
+}
+
@DeveloperApi
-class StorageStatusListener extends SparkListener {
+class StorageStatusListener(conf: SparkConf) extends SparkListener {
+ var ticker = Ticker.systemTicker()
+
+ private [storage] def this(conf: SparkConf, ticker: Ticker) = {
+this(conf)
+this.ticker = ticker
+ }
+
+ import StorageStatusListener._
+
// This maintains only blocks that are cached (i.e. storage level is not
StorageLevel.NONE)
private[storage] val executorIdToStorageStatus = mutable.Map[String,
StorageStatus]()
+ private[storage] val removedExecutorIdToStorageStatus =
CacheBuilder.newBuilder().
+expireAfterWrite(conf.getTimeAsSeconds(TIME_TO_EXPIRE_KILLED_EXECUTOR,
"0"), TimeUnit.SECONDS).
+ticker(ticker).build[String, StorageStatus]()
def storageStatusList: Seq[StorageStatus] = synchronized {
executorIdToStorageStatus.values.toSeq
}
-
+
+ def removedExecutorStorageStatusList: Seq[StorageStatus] = synchronized{
--- End diff --
nit: space before {
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44063735
--- Diff:
core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala
---
@@ -150,4 +157,21 @@ class StorageStatusListenerSuite extends FunSuite {
listener.onUnpersistRDD(SparkListenerUnpersistRDD(1))
assert(listener.executorIdToStorageStatus("big").numBlocks === 0)
}
+
+ test("Killed Executor Entry removed after configurable time") {
+val localtestconf = new
SparkConf().set(StorageStatusListener.TIME_TO_EXPIRE_KILLED_EXECUTOR,"5s")
+val ticker = new Ticker {
+ val nanos = new AtomicLong()
+ def advance(time: Long, timeUnit: TimeUnit) = {
+nanos.addAndGet(timeUnit.toNanos(time))
+ }
+ override def read() = {
+nanos.getAndAdd(0)
+ }
+}
+val listener = new StorageStatusListener(localtestconf, ticker)
+listener.removedExecutorIdToStorageStatus.put("1", new
StorageStatus(null, 50))
+ticker.advance(5, TimeUnit.SECONDS)
+assert(listener.removedExecutorIdToStorageStatus.asMap.get("1") ==
null)
--- End diff --
this is more complicated than it needs to be -- no need for an atomic
(there is only one thread here) you can just use a long. also I'd check the
`removedExecutorStorageStatusList` method, rather than the cache itself.
```scala
class MyTicker extends Ticker {
var t = 0L
override def read(): Long = t
}
val ticker = new MyTicker
val listener = new StorageStatusListener(localtestconf, ticker)
listener.removedExecutorIdToStorageStatus.put("1", new
StorageStatus(null, 50))
assert(listener.removedExecutorStorageStatusList.nonEmpty)
ticker.t = 51L
assert(listener.removedExecutorStorageStatusList.isEmpty)
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-154179267 Jenkins, ok to test --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44055259 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -17,26 +17,55 @@ package org.apache.spark.storage +import java.util.concurrent.TimeUnit + +import scala.collection.JavaConversions.collectionAsScalaIterable import scala.collection.mutable +import scala.language.reflectiveCalls +import org.apache.spark.SparkConf import org.apache.spark.annotation.DeveloperApi import org.apache.spark.scheduler._ +import com.google.common.base.Ticker +import com.google.common.cache.CacheBuilder --- End diff -- import ordering -- these go above the o.a.spark imports, with a linebreak in between. See https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide#SparkCodeStyleGuide-Imports --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44059314
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -17,26 +17,55 @@
package org.apache.spark.storage
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConversions.collectionAsScalaIterable
import scala.collection.mutable
+import scala.language.reflectiveCalls
+import org.apache.spark.SparkConf
import org.apache.spark.annotation.DeveloperApi
import org.apache.spark.scheduler._
+import com.google.common.base.Ticker
+import com.google.common.cache.CacheBuilder
+
/**
* :: DeveloperApi ::
* A SparkListener that maintains executor storage status.
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
+}
+
@DeveloperApi
-class StorageStatusListener extends SparkListener {
+class StorageStatusListener(conf: SparkConf) extends SparkListener {
+ var ticker = Ticker.systemTicker()
+
+ private [storage] def this(conf: SparkConf, ticker: Ticker) = {
+this(conf)
+this.ticker = ticker
+ }
+
+ import StorageStatusListener._
+
// This maintains only blocks that are cached (i.e. storage level is not
StorageLevel.NONE)
private[storage] val executorIdToStorageStatus = mutable.Map[String,
StorageStatus]()
+ private[storage] val removedExecutorIdToStorageStatus =
CacheBuilder.newBuilder().
+expireAfterWrite(conf.getTimeAsSeconds(TIME_TO_EXPIRE_KILLED_EXECUTOR,
"0"), TimeUnit.SECONDS).
+ticker(ticker).build[String, StorageStatus]()
def storageStatusList: Seq[StorageStatus] = synchronized {
executorIdToStorageStatus.values.toSeq
}
-
+
+ def removedExecutorStorageStatusList: Seq[StorageStatus] = synchronized{
--- End diff --
Thanks for pointing it out. Corrected.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44059300 --- Diff: core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala --- @@ -17,10 +17,16 @@ package org.apache.spark.storage -import org.scalatest.FunSuite +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicLong + +import org.apache.spark.SparkConf import org.apache.spark.Success import org.apache.spark.executor.TaskMetrics import org.apache.spark.scheduler._ +import org.scalatest.FunSuite + +import com.google.common.base.Ticker --- End diff -- Thanks for pointing it out. Corrected. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44059357
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -17,26 +17,55 @@
package org.apache.spark.storage
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConversions.collectionAsScalaIterable
import scala.collection.mutable
+import scala.language.reflectiveCalls
+import org.apache.spark.SparkConf
import org.apache.spark.annotation.DeveloperApi
import org.apache.spark.scheduler._
+import com.google.common.base.Ticker
+import com.google.common.cache.CacheBuilder
+
/**
* :: DeveloperApi ::
* A SparkListener that maintains executor storage status.
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
+}
+
@DeveloperApi
-class StorageStatusListener extends SparkListener {
+class StorageStatusListener(conf: SparkConf) extends SparkListener {
+ var ticker = Ticker.systemTicker()
+
+ private [storage] def this(conf: SparkConf, ticker: Ticker) = {
+this(conf)
+this.ticker = ticker
+ }
--- End diff --
Yes, This definitely looks better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44063828 --- Diff: core/src/test/scala/org/apache/spark/ui/storage/StorageTabSuite.scala --- @@ -22,6 +22,7 @@ import org.apache.spark.Success import org.apache.spark.executor.TaskMetrics import org.apache.spark.scheduler._ import org.apache.spark.storage._ +import org.apache.spark.SparkConf --- End diff -- nit: import ordering, should go above the rest of the spark imports (b/c class imports sort before package imports). Also while you're touching this, there should be a blank line between the scalatest and spark imports --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44055138 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -17,26 +17,55 @@ package org.apache.spark.storage +import java.util.concurrent.TimeUnit + +import scala.collection.JavaConversions.collectionAsScalaIterable import scala.collection.mutable +import scala.language.reflectiveCalls --- End diff -- unused? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r44055116 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -17,26 +17,55 @@ package org.apache.spark.storage +import java.util.concurrent.TimeUnit + +import scala.collection.JavaConversions.collectionAsScalaIterable --- End diff -- avoid using JavaConversions, you should prefer JavaConverters, which forces you to call `.asScala`, making the transformation much clearer to future code readers. the convention is to `import scala.collection.JavaConverters._` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44064522
--- Diff:
core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala
---
@@ -150,4 +157,21 @@ class StorageStatusListenerSuite extends FunSuite {
listener.onUnpersistRDD(SparkListenerUnpersistRDD(1))
assert(listener.executorIdToStorageStatus("big").numBlocks === 0)
}
+
+ test("Killed Executor Entry removed after configurable time") {
+val localtestconf = new
SparkConf().set(StorageStatusListener.TIME_TO_EXPIRE_KILLED_EXECUTOR,"5s")
--- End diff --
nit: line too long
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44064729
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -28,15 +36,33 @@ import org.apache.spark.scheduler._
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
+}
+
@DeveloperApi
-class StorageStatusListener extends SparkListener {
+class StorageStatusListener private[storage](conf: SparkConf, ticker:
Ticker) {
+ def this(conf: SparkConf) = {
+this(conf, Ticker.systemTicker())
+ }
+
+ import StorageStatusListener._
+
// This maintains only blocks that are cached (i.e. storage level is not
StorageLevel.NONE)
private[storage] val executorIdToStorageStatus = mutable.Map[String,
StorageStatus]()
+ private[storage] val removedExecutorIdToStorageStatus =
CacheBuilder.newBuilder().
+expireAfterWrite(conf.getTimeAsSeconds(TIME_TO_EXPIRE_KILLED_EXECUTOR,
"0"), TimeUnit.SECONDS).
+ticker(ticker).build[String, StorageStatus]()
--- End diff --
super nit:
can you put each chained call on its own line, with the `.` at the
beginning of the line? so
```scala
private[storage] val removedExecutorIdToStorageStatus =
CacheBuilder.newBuilder()
.expireAfterWrite(...)
.ticker(ticker)
.build...
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r44064794
--- Diff:
core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala ---
@@ -28,15 +36,33 @@ import org.apache.spark.scheduler._
*
* This class is thread-safe (unlike JobProgressListener)
*/
+
+object StorageStatusListener {
+ val TIME_TO_EXPIRE_KILLED_EXECUTOR =
"spark.ui.timeToExpireKilledExecutor"
+}
+
@DeveloperApi
-class StorageStatusListener extends SparkListener {
+class StorageStatusListener private[storage](conf: SparkConf, ticker:
Ticker) {
+ def this(conf: SparkConf) = {
+this(conf, Ticker.systemTicker())
+ }
+
+ import StorageStatusListener._
+
// This maintains only blocks that are cached (i.e. storage level is not
StorageLevel.NONE)
private[storage] val executorIdToStorageStatus = mutable.Map[String,
StorageStatus]()
+ private[storage] val removedExecutorIdToStorageStatus =
CacheBuilder.newBuilder().
+expireAfterWrite(conf.getTimeAsSeconds(TIME_TO_EXPIRE_KILLED_EXECUTOR,
"0"), TimeUnit.SECONDS).
+ticker(ticker).build[String, StorageStatus]()
--- End diff --
also does this need to be `private[storage]`, or can it just be `private`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r43982245
--- Diff:
core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala
---
@@ -150,4 +158,12 @@ class StorageStatusListenerSuite extends FunSuite {
listener.onUnpersistRDD(SparkListenerUnpersistRDD(1))
assert(listener.executorIdToStorageStatus("big").numBlocks === 0)
}
+
+ test("Killed Executor Entry removed after configurable time") {
+val localtestconf = new
SparkConf().set(StorageStatusListener.TIME_TO_EXPIRE_KILLED_EXECUTOR,"5s")
+val listener = new StorageStatusListener(localtestconf)
+listener.removedExecutorIdToStorageStatus.put("1", new
StorageStatus(null, 50))
+Thread.sleep(5500)
--- End diff --
For that we'll have to set an arbitrary ticker to the main cache, We would
not want to set any arbitrary ticker to the original cache. Creating a new
cache in the test would not be testing of our functionality and would be
equivalent of testing just the Guava Cache's code. right?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r43909655 --- Diff: core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala --- @@ -21,6 +21,13 @@ import org.scalatest.FunSuite import org.apache.spark.Success import org.apache.spark.executor.TaskMetrics import org.apache.spark.scheduler._ +import org.apache.spark.SparkConfSuite +import org.apache.spark.SparkConf + +/** + * Test the behavior of StorageStatusListener in response to all relevant events. + */ + --- End diff -- spurious extra doc, I don't think you use `SparkConfSuite`, and the import of `SparkConf` is out of order --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/6263#discussion_r43909507
--- Diff:
core/src/test/scala/org/apache/spark/storage/StorageStatusListenerSuite.scala
---
@@ -150,4 +158,12 @@ class StorageStatusListenerSuite extends FunSuite {
listener.onUnpersistRDD(SparkListenerUnpersistRDD(1))
assert(listener.executorIdToStorageStatus("big").numBlocks === 0)
}
+
+ test("Killed Executor Entry removed after configurable time") {
+val localtestconf = new
SparkConf().set(StorageStatusListener.TIME_TO_EXPIRE_KILLED_EXECUTOR,"5s")
+val listener = new StorageStatusListener(localtestconf)
+listener.removedExecutorIdToStorageStatus.put("1", new
StorageStatus(null, 50))
+Thread.sleep(5500)
--- End diff --
you can avoid sleeping by using
[`cache.setTicker`](https://github.com/google/guava/wiki/CachesExplained#testing-timed-eviction)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/6263#discussion_r43909732 --- Diff: core/src/main/scala/org/apache/spark/storage/StorageStatusListener.scala --- @@ -18,9 +18,14 @@ package org.apache.spark.storage import scala.collection.mutable - import org.apache.spark.annotation.DeveloperApi import org.apache.spark.scheduler._ +import com.google.common.cache.CacheBuilder +import java.util.concurrent.TimeUnit +import com.google.common.cache.CacheLoader +import org.apache.spark.SparkConf + +import scala.collection.JavaConversions._ --- End diff -- nit: import ordering --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-153674563 @suyanNone Can you please review my 2nd commit. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user suyanNone commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-153225204 yean, make it configurable looks good --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user archit279thakur commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-151574473 Sure, and time for expiration should be configuration based? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user suyanNone commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-151010358 Hi, @archit279thakur would you mind add the logic about adding a time expire to show lost-Executor log? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-148821303 It looks like this PR and #6644 duplicate / overlap with each other. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-121072677 Can one of the admins verify this patch? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
GitHub user archit279thakur opened a pull request: https://github.com/apache/spark/pull/6263 SPARK-7729:Executor which has been killed should also be displayed on⦠⦠Executors Tab. You can merge this pull request into a Git repository by running: $ git pull https://github.com/archit279thakur/spark SPARK-7729 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/6263.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 #6263 commit 7a82254a12153d8cdd2d2ed1bb14843e1241f63e Author: archit.thakur archit.tha...@guavus.com Date: 2015-05-19T11:55:35Z SPARK-7729:Executor which has been killed should also be displayed on Executors Tab. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: SPARK-7729:Executor which has been killed shou...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6263#issuecomment-103462304 Can one of the admins verify this patch? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
