[GitHub] spark pull request: [SPARK-4012] call tryOrExit instead of logUnca...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-62259510 Hi, @andrewor14 , Sorry about the misunderstanding and thanks for your patient explanation I will resubmit it and hopefully will get more feedbacks about this, 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-4012] call tryOrExit instead of logUnca...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/2864 --- 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-4012] call tryOrExit instead of logUnca...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-62243302 OKI didn't expect the PR is closed in such way (for this PR, I'm obviously waiting for more responses and is trying to get the clearer answer from the @andrewor14 that whether I misunderstood something or we shall propagate the chances to other places, see my last comment) just a suggestion. closing PRs without active maintenance is definitely a good way to keep the repository clean. However, I think maybe the better way to close some pending pull request is to end the discussion first by getting some answer, instead of terminating the discussion without any pre-notification and suddenly close an actual active PR I'm not intended to be offending. @andrewor14 is making great contributions to the community...what I talked about is just for making the review process more reasonable --- 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-4012] call tryOrExit instead of logUnca...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-62244475 Hey @CodingCat sorry this is unintended. The automatic closing of pull requests is searches for key words that include mind closing this, which I said a while ago before we started discussing the specifics. I have been swamped with the 1.2 release so I haven't had the time to look at some older PRs that are not as urgent, but I definitely did not intend to just terminate the discussion so abruptly. If you wish to open a new PR on the same issue, please feel free to do so. Personally, I am ambivalent on whether this particular issue is a good change, but maybe others can look at it. --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-61476280 [Test build #22813 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22813/consoleFull) for PR 2864 at commit [`39eeb02`](https://github.com/apache/spark/commit/39eeb02ed68598678bad17134ca47b3b05571c57). * This patch merges cleanly. --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-61488002 [Test build #22813 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22813/consoleFull) for PR 2864 at commit [`39eeb02`](https://github.com/apache/spark/commit/39eeb02ed68598678bad17134ca47b3b05571c57). * 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-4012] call tryOrExit instead of logUnca...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-61488015 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22813/ Test PASSed. --- 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-4012] call tryOrExit instead of logUnca...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-61489536 Hey, @andrewor14 , Any further thoughts about this PR? --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60452574 [Test build #22166 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22166/consoleFull) for PR 2864 at commit [`e163adf`](https://github.com/apache/spark/commit/e163adf42644561e1ecfbf9e16e15d9a5c6e3428). * This patch merges cleanly. --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60457785 [Test build #22166 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22166/consoleFull) for PR 2864 at commit [`e163adf`](https://github.com/apache/spark/commit/e163adf42644561e1ecfbf9e16e15d9a5c6e3428). * 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-4012] call tryOrExit instead of logUnca...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60457792 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22166/ 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-4012] call tryOrExit instead of logUnca...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60314190 I see... when the JVM's `UncaughtExceptionHandler` catches an OOM exception it doesn't actually kill the JVM. However, we do want to log the fact that we are facing an OOM exception somewhere. Even though `ExecutorUncaughtExceptionHandler` eventually does that it's not clear from the name `tryOrExit` that we will log the error. I'm just not sure how badly this issue needs to be fixed. We do this in other places too (e.g. LiveListenerBus has a polling thread). --- 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-4012] call tryOrExit instead of logUnca...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60316623 @andrewor14, (just met a LiveListernBus uncaught exception this afternoon) personally, I feel that we shall stop the driver when such things happen ... e.g. the user may need to checkout the JVM process liveness for , for instance, HA, after this uncaught exception is thrown, the JVM process is alive but not functional If we agree on that...I think maybe we need to propagate the changes to other places --- 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-4012] call tryOrExit instead of logUnca...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60174247 Hey @CodingCat I think this is the expected behavior. If an OOM is thrown on the driver then the context cleaner thread should just die. In fact `logUncaughtException` rethrows the error and this is propagated to Java's `UncaughtExceptionHandler`. I don't think we want to use `tryOrExit` here because we want to log the exception, and we don't want to use the `ExecutorUncaughtExceptionHandler` for the driver. Would you mind closing 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-4012] call tryOrExit instead of logUnca...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60179940 Hi, @andrewor14, the issue here is JVM default UncaughtExceptionHandler seems not handle the exception correctly, as I said in the PR description, it will request user to shutdown JVM manually w.r.t use ExecutorUncaughtExceptionHandler in driver side, it has been there for a while..https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/TaskSchedulerImpl.scala#L148 --- 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-4012] call tryOrExit instead of logUnca...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60181397 this is also very similar to https://github.com/apache/spark/pull/622/, where the main thread cannot handle the exception thrown by the akka's scheduler thread --- 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-4012] call tryOrExit instead of logUnca...
Github user CodingCat commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-60181601 but I don't mind to grab that ExecutorUncaughtExceptionHandler to somewhere else to make it more general --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59911943 [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21985/consoleFull) for PR 2864 at commit [`2f5a4c0`](https://github.com/apache/spark/commit/2f5a4c0070f054d87a96fe5524416460e67e0d24). * This patch merges cleanly. --- 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-4012] call tryOrExit instead of logUnca...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59924326 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21985/ 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59924317 **[Tests timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21985/consoleFull)** for PR 2864 at commit [`2f5a4c0`](https://github.com/apache/spark/commit/2f5a4c0070f054d87a96fe5524416460e67e0d24) after a configured wait of `120m`. --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59982711 [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21999/consoleFull) for PR 2864 at commit [`737d36b`](https://github.com/apache/spark/commit/737d36b2bb0202ab7982a7874d1e31bc66091581). * This patch merges cleanly. --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59993293 [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21999/consoleFull) for PR 2864 at commit [`737d36b`](https://github.com/apache/spark/commit/737d36b2bb0202ab7982a7874d1e31bc66091581). * 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-4012] call tryOrExit instead of logUnca...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59993306 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21999/ Test PASSed. --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59871965 [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21958/consoleFull) for PR 2864 at commit [`3893a7e`](https://github.com/apache/spark/commit/3893a7e051674df70124b09c386c13afdc5ab3d8). * This patch merges cleanly. --- 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-4012] call tryOrExit instead of logUnca...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59875799 [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21958/consoleFull) for PR 2864 at commit [`3893a7e`](https://github.com/apache/spark/commit/3893a7e051674df70124b09c386c13afdc5ab3d8). * This patch **fails PySpark unit tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `class JavaFutureActionWrapper[S, T](futureAction: FutureAction[S], converter: S = T)` * ` case class ReconnectWorker(masterUrl: String) extends DeployMessage` --- 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-4012] call tryOrExit instead of logUnca...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/2864#issuecomment-59875800 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21958/ 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
