[GitHub] spark pull request: [SPARK-12755][CORE] Stop the event logger befo...
Github user mallman commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-174599806 Thanks, @srowen. --- 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-12755][CORE] Stop the event logger befo...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/10700 --- 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-12755][CORE] Stop the event logger befo...
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-174454362 Merged to master/1.6/1.5 --- 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-12755][CORE] Stop the event logger befo...
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-173505157 Are there downsides to merging this to master, even if the related functionality is about to be removed? it passes tests, and seems to improve an ordering of shutdown, and can be backported to fix an actual minor issue in previous releases. Tests would be cool but you're correct that this one could be really hard to trigger. I see no reason to close 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-12755][CORE] Stop the event logger befo...
Github user mallman commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-173490509 Here are my current thoughts. Josh says this functionality is going to be removed in Spark 2.0. The bug this PR is designed to address manifests itself in Spark 1.5 in three ways (I'm aware of): 1. Misleading log messages from the Master (reported above). 2. Incomplete (aka "in progress") application event logs, which can be further divided into two scenarios: 2.a. Incomplete uncompressed event log files. The log processor can recover these files. 2.b. Incomplete compressed event log files. The compression output is truncated and unreadable by normal means. The history server reports a corrupted event log. I cannot definitively tie that symptom to this bug, but it agrees with my experience. The most problematic of these is unrecoverable event logs. I've been frustrated by this before and turned off event log compression as a workaround. Since deploying a build with this patch to one of our dev clusters I haven't seen this problem again. I don't see a simple way to write a test to support this PR. Overall, I feel we should close this PR but keep a reference to it from Jira with a comment that Spark 1.5 and 1.6 users can try this patchâat their own riskâto address the described symptoms if they wish to. It's going into our own Spark 1.x builds. I'll close this PR and the associated Jira issue within the next few days unless someone objects or wishes to continue discussion. 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-12755][CORE] Stop the event logger befo...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172608176 **[Test build #2398 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2398/consoleFull)** for PR 10700 at commit [`57cade1`](https://github.com/apache/spark/commit/57cade176fa176d6642a78e2f16430ffdca19a6e). * This patch passes all 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-12755][CORE] Stop the event logger befo...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172574018 **[Test build #2398 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2398/consoleFull)** for PR 10700 at commit [`57cade1`](https://github.com/apache/spark/commit/57cade176fa176d6642a78e2f16430ffdca19a6e). --- 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-12755][CORE] Stop the event logger befo...
Github user mallman commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172564021 Sorry guys. I bungled the ordering of the `stop()` calls. That's what I get for doing a manual patch from a manual diff from another branch-1.5... :disappointed: This patch passes all tests on branch-1.3. Can you please kick off a new test build in jenkins? --- 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-12755][CORE] Stop the event logger befo...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172557308 **[Test build #2397 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2397/consoleFull)** for PR 10700 at commit [`6c32f77`](https://github.com/apache/spark/commit/6c32f77890d8ac5f2117020a65167e27124aeae5). * This patch **fails Spark unit 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-12755][CORE] Stop the event logger befo...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172530317 **[Test build #2397 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2397/consoleFull)** for PR 10700 at commit [`6c32f77`](https://github.com/apache/spark/commit/6c32f77890d8ac5f2117020a65167e27124aeae5). --- 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-12755][CORE] Stop the event logger befo...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172503953 **[Test build #2395 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2395/consoleFull)** for PR 10700 at commit [`6c32f77`](https://github.com/apache/spark/commit/6c32f77890d8ac5f2117020a65167e27124aeae5). * This patch **fails Spark unit 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-12755][CORE] Stop the event logger befo...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172484488 **[Test build #2395 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2395/consoleFull)** for PR 10700 at commit [`6c32f77`](https://github.com/apache/spark/commit/6c32f77890d8ac5f2117020a65167e27124aeae5). --- 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-12755][CORE] Stop the event logger befo...
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172483987 Yes, we'd prefer to make changes in `master` and then back port rather than apply only to a branch. I think the change is at worst a no-op for `master`? if so I think this change is fine. It sounds like it needs to be back-ported to 1.5. --- 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-12755][CORE] Stop the event logger befo...
Github user mallman commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172443532 I should also state that my original motivation in submitting this patch was to address the confusing log messages Application ... is still in progress, it may be terminated abnormally. which I saw in the Spark master log for app's that actually terminated normally. Also, it's just come to mind that this bug may explain another behavior I've seenâsometimes an app's event log is corrupted if it was configured to be compressed. If the log is uncompressed then the ability for the history reader to decode an "in progress" event log allows it to be processed as expected. However, if the event log is being written through a compressed output stream and is not properly flushed before it is processed, then the processing may fail because the file compression was incomplete. (As this just occurred to me I haven't tested this hypothesis, but it does sound like a plausible explanation.) If this is the case, then this patch should correct the problem with corrupt compressed event logs. --- 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-12755][CORE] Stop the event logger befo...
Github user mallman commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-172440507 Hi Josh, Good questions. I may have submitted this PR incorrectly. Perhaps you can guide me in the right direction. I submitted this PR for merging into master because my understanding is that's how all PR's for the Spark project should be created. And patches against master may be backported to earlier releases. However, I originally created and tested this patch on branch-1.5 because that's what we're currently running. So while this patch may be irrelevant to master (or Spark 2.0), it's relevant to the Spark 1.5 branch and presumably 1.6 as well. Under these circumstances, should I have submitted a PR against master as I have done? The code contribution guidelines state that only in a special case would a PR be opened against another branch. Does a patch with no or lesser relevance to the master branch compared to an earlier release branch qualify as a "special case"? And if so, which branch should I have submitted the PR against? 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-12755][CORE] Stop the event logger befo...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/10700#issuecomment-171442415 Will this change still be relevant if we remove the Spark Master's embedded HistoryServer? In other words, does this race condition affect the standalone HistoryServer or only the Master history server? If it only affects the master then this isn't worth changing in master since we're going to remove the master's embedded history server for Spark 2.0. It may still be worthwhile for Spark 1.6, though, although there's a risk/benefit trade-off here. --- 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-12755][CORE] Stop the event logger befo...
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10700#discussion_r49654293
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -1681,6 +1681,9 @@ class SparkContext(config: SparkConf) extends Logging
with ExecutorAllocationCli
Utils.tryLogNonFatalError {
_executorAllocationManager.foreach(_.stop())
}
+Utils.tryLogNonFatalError {
--- End diff --
Maybe add a comment here to explain why this needs to be stopped before the
DAGScheduler in order to prevent this change from being accidentally lost in
the future?
---
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
