You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zsxwing <gi...@git.apache.org> on 2015/10/14 15:49:59 UTC
[GitHub] spark pull request: Fix a deadlock in StreamingContex.stop
GitHub user zsxwing opened a pull request:
https://github.com/apache/spark/pull/9116
Fix a deadlock in StreamingContex.stop
The following deadlock may happen if shutdownHook and StreamingContext.stop are running at the same time.
```
Java stack information for the threads listed above:
===================================================
"Thread-2":
at org.apache.spark.streaming.StreamingContext.stop(StreamingContext.scala:699)
- waiting to lock <0x00000005405a1680> (a org.apache.spark.streaming.StreamingContext)
at org.apache.spark.streaming.StreamingContext.org$apache$spark$streaming$StreamingContext$$stopOnShutdown(StreamingContext.scala:729)
at org.apache.spark.streaming.StreamingContext$$anonfun$start$1.apply$mcV$sp(StreamingContext.scala:625)
at org.apache.spark.util.SparkShutdownHook.run(ShutdownHookManager.scala:266)
at org.apache.spark.util.SparkShutdownHookManager$$anonfun$runAll$1$$anonfun$apply$mcV$sp$1.apply$mcV$sp(ShutdownHookManager.scala:236)
at org.apache.spark.util.SparkShutdownHookManager$$anonfun$runAll$1$$anonfun$apply$mcV$sp$1.apply(ShutdownHookManager.scala:236)
at org.apache.spark.util.SparkShutdownHookManager$$anonfun$runAll$1$$anonfun$apply$mcV$sp$1.apply(ShutdownHookManager.scala:236)
at org.apache.spark.util.Utils$.logUncaughtExceptions(Utils.scala:1697)
at org.apache.spark.util.SparkShutdownHookManager$$anonfun$runAll$1.apply$mcV$sp(ShutdownHookManager.scala:236)
at org.apache.spark.util.SparkShutdownHookManager$$anonfun$runAll$1.apply(ShutdownHookManager.scala:236)
at org.apache.spark.util.SparkShutdownHookManager$$anonfun$runAll$1.apply(ShutdownHookManager.scala:236)
at scala.util.Try$.apply(Try.scala:161)
at org.apache.spark.util.SparkShutdownHookManager.runAll(ShutdownHookManager.scala:236)
- locked <0x00000005405b6a00> (a org.apache.spark.util.SparkShutdownHookManager)
at org.apache.spark.util.SparkShutdownHookManager$$anon$2.run(ShutdownHookManager.scala:216)
at org.apache.hadoop.util.ShutdownHookManager$1.run(ShutdownHookManager.java:54)
"main":
at org.apache.spark.util.SparkShutdownHookManager.remove(ShutdownHookManager.scala:248)
- waiting to lock <0x00000005405b6a00> (a org.apache.spark.util.SparkShutdownHookManager)
at org.apache.spark.util.ShutdownHookManager$.removeShutdownHook(ShutdownHookManager.scala:199)
at org.apache.spark.streaming.StreamingContext.stop(StreamingContext.scala:712)
- locked <0x00000005405a1680> (a org.apache.spark.streaming.StreamingContext)
at org.apache.spark.streaming.StreamingContext.stop(StreamingContext.scala:684)
- locked <0x00000005405a1680> (a org.apache.spark.streaming.StreamingContext)
at org.apache.spark.streaming.SessionByKeyBenchmark$.main(SessionByKeyBenchmark.scala:108)
at org.apache.spark.streaming.SessionByKeyBenchmark.main(SessionByKeyBenchmark.scala)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at org.apache.spark.deploy.SparkSubmit$.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:680)
at org.apache.spark.deploy.SparkSubmit$.doRunMain$1(SparkSubmit.scala:180)
at org.apache.spark.deploy.SparkSubmit$.submit(SparkSubmit.scala:205)
at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:120)
at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala)
```
This PR just moved `ShutdownHookManager.removeShutdownHook` out of `synchronized` to avoid deadlock.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zsxwing/spark stop-deadlock
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/9116.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 #9116
----
commit abd21c6a875a9d92e6489d44ec5898949770839d
Author: zsxwing <zs...@gmail.com>
Date: 2015-10-14T13:41:36Z
Fix a deadlock in StreamingContex.stop
The deadlock may happen if shutdownHook and StreamingContext.stop are running at the same time.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148310651
[Test build #43777 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43777/consoleFull) for PR 9116 at commit [`7c44b8c`](https://github.com/apache/spark/commit/7c44b8c2db3b6b785705c9dccca92e2a8a8fce9d).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r41998454
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
--- End diff --
Looks good, but do you need a second var here? just move the shutting-down part outside the synchronized block.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148310218
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148635713
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43833/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148619226
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148635564
[Test build #43833 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43833/console) for PR 9116 at commit [`f6440b6`](https://github.com/apache/spark/commit/f6440b62178360f8f9964bf0820f35b59632e399).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42010758
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
--- End diff --
Yes I get that; was mostly wondering whether a second variable was needed at all. I think it is, to fully preserve the way it works now. This seems OK.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148321661
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43777/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148615212
I dont think the possible deadlocks are not fixed yet, as sparkContext.stop() will try to lock the same that we hit when we try to remove StreamingContext's shutdown hook.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148321535
[Test build #43777 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43777/console) for PR 9116 at commit [`7c44b8c`](https://github.com/apache/spark/commit/7c44b8c2db3b6b785705c9dccca92e2a8a8fce9d).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148358633
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148074156
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43719/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148073949
[Test build #43719 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43719/console) for PR 9116 at commit [`abd21c6`](https://github.com/apache/spark/commit/abd21c6a875a9d92e6489d44ec5898949770839d).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148619099
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148055889
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148635710
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148371226
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43786/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148358652
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42210175
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
+ synchronized {
+ try {
+ state match {
+ case INITIALIZED =>
+ logWarning("StreamingContext has not been started yet")
+ case STOPPED =>
+ logWarning("StreamingContext has already been stopped")
+ case ACTIVE =>
+ scheduler.stop(stopGracefully)
+ // Removing the streamingSource to de-register the metrics on stop()
+ env.metricsSystem.removeSource(streamingSource)
+ uiTab.foreach(_.detach())
+ StreamingContext.setActiveContext(null)
+ waiter.notifyStop()
+ if (shutdownHookRef != null) {
+ shutdownHookRefToRemove = shutdownHookRef
+ shutdownHookRef = null
+ }
+ logInfo("StreamingContext stopped successfully")
+ }
+ // Even if we have already stopped, we still need to attempt to stop the SparkContext
+ // because a user might stop(stopSparkContext = false) and then call
+ // stop(stopSparkContext = true).
+ if (stopSparkContext) sc.stop()
--- End diff --
Might be a good idea to move this to outside synchronized 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148371080
[Test build #43786 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43786/console) for PR 9116 at commit [`8060321`](https://github.com/apache/spark/commit/806032197ebd7494dd567482778174cf74be2645).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148310203
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148356952
@srowen Agree. I prefer to narrow the change of this PR, too. Reverted my previous 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148345442
Hm, this is getting unwieldy. There are several nested try blocks here. The same argument goes for many of these methods -- if one fails should they not continue trying? A more tidy solution would be to execute a series of `() -> Unit` code blocks that perform some cleanup and make sure that they each fire in succession, regardless of the others. The final one to remove the shutdown hook could occur outside synchronization.
I realize we're expanding the scope of the change here, but is it maybe worthwhile to go all the way here?
Really, something similar could be done for `SparkContext` and there's an existing JIRA for it somewhere.
At least, I'd prefer to either narrowly fix the deadlock here, or fix all of the `finally`-related issue separately and all at once.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42084412
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
+ synchronized {
+ try {
+ state match {
+ case INITIALIZED =>
+ logWarning("StreamingContext has not been started yet")
+ case STOPPED =>
+ logWarning("StreamingContext has already been stopped")
+ case ACTIVE =>
+ scheduler.stop(stopGracefully)
+ // Removing the streamingSource to de-register the metrics on stop()
+ env.metricsSystem.removeSource(streamingSource)
+ uiTab.foreach(_.detach())
+ StreamingContext.setActiveContext(null)
+ waiter.notifyStop()
+ if (shutdownHookRef != null) {
+ shutdownHookRefToRemove = shutdownHookRef
+ shutdownHookRef = null
+ }
+ logInfo("StreamingContext stopped successfully")
+ }
+ // Even if we have already stopped, we still need to attempt to stop the SparkContext
+ // because a user might stop(stopSparkContext = false) and then call
+ // stop(stopSparkContext = true).
+ if (stopSparkContext) sc.stop()
--- End diff --
This is a different shut down hook in StreamingContext.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148057871
[Test build #43719 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43719/consoleFull) for PR 9116 at commit [`abd21c6`](https://github.com/apache/spark/commit/abd21c6a875a9d92e6489d44ec5898949770839d).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148371225
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148055858
Merged 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148625717
[Test build #43833 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43833/consoleFull) for PR 9116 at commit [`f6440b6`](https://github.com/apache/spark/commit/f6440b62178360f8f9964bf0820f35b59632e399).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42080466
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
+ synchronized {
+ try {
+ state match {
+ case INITIALIZED =>
+ logWarning("StreamingContext has not been started yet")
+ case STOPPED =>
+ logWarning("StreamingContext has already been stopped")
+ case ACTIVE =>
+ scheduler.stop(stopGracefully)
+ // Removing the streamingSource to de-register the metrics on stop()
+ env.metricsSystem.removeSource(streamingSource)
+ uiTab.foreach(_.detach())
+ StreamingContext.setActiveContext(null)
+ waiter.notifyStop()
+ if (shutdownHookRef != null) {
+ shutdownHookRefToRemove = shutdownHookRef
+ shutdownHookRef = null
+ }
+ logInfo("StreamingContext stopped successfully")
+ }
+ // Even if we have already stopped, we still need to attempt to stop the SparkContext
+ // because a user might stop(stopSparkContext = false) and then call
+ // stop(stopSparkContext = true).
+ if (stopSparkContext) sc.stop()
+ } finally {
+ // The state should always be Stopped after calling `stop()`, even if we haven't started yet
+ state = STOPPED
}
- // Even if we have already stopped, we still need to attempt to stop the SparkContext because
- // a user might stop(stopSparkContext = false) and then call stop(stopSparkContext = true).
- if (stopSparkContext) sc.stop()
- } finally {
- // The state should always be Stopped after calling `stop()`, even if we haven't started yet
- state = STOPPED
+ }
+ if (shutdownHookRefToRemove != null) {
--- End diff --
should this be inside a finally {}? what if, say sc.stop() throws? wouldn't this leak the shutdownhookreftoremove?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148550760
https://issues.apache.org/jira/browse/SPARK-11137
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148618704
> I dont think the possible deadlocks are not fixed yet, as sparkContext.stop() will try to lock the same that we hit when we try to remove StreamingContext's shutdown hook.
:+1: Fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42092604
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
+ synchronized {
+ try {
+ state match {
+ case INITIALIZED =>
+ logWarning("StreamingContext has not been started yet")
+ case STOPPED =>
+ logWarning("StreamingContext has already been stopped")
+ case ACTIVE =>
+ scheduler.stop(stopGracefully)
+ // Removing the streamingSource to de-register the metrics on stop()
+ env.metricsSystem.removeSource(streamingSource)
+ uiTab.foreach(_.detach())
+ StreamingContext.setActiveContext(null)
+ waiter.notifyStop()
+ if (shutdownHookRef != null) {
+ shutdownHookRefToRemove = shutdownHookRef
+ shutdownHookRef = null
+ }
+ logInfo("StreamingContext stopped successfully")
+ }
+ // Even if we have already stopped, we still need to attempt to stop the SparkContext
+ // because a user might stop(stopSparkContext = false) and then call
+ // stop(stopSparkContext = true).
+ if (stopSparkContext) sc.stop()
+ } finally {
+ // The state should always be Stopped after calling `stop()`, even if we haven't started yet
+ state = STOPPED
}
- // Even if we have already stopped, we still need to attempt to stop the SparkContext because
- // a user might stop(stopSparkContext = false) and then call stop(stopSparkContext = true).
- if (stopSparkContext) sc.stop()
- } finally {
- // The state should always be Stopped after calling `stop()`, even if we haven't started yet
- state = STOPPED
+ }
+ if (shutdownHookRefToRemove != null) {
--- End diff --
Yeah, I put them to `finally`.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148648995
LGTM. Merging this is in master as well as branch 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148074154
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by tdas <gi...@git.apache.org>.
Github user tdas commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42209868
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
+ synchronized {
+ try {
+ state match {
+ case INITIALIZED =>
+ logWarning("StreamingContext has not been started yet")
+ case STOPPED =>
+ logWarning("StreamingContext has already been stopped")
+ case ACTIVE =>
+ scheduler.stop(stopGracefully)
+ // Removing the streamingSource to de-register the metrics on stop()
+ env.metricsSystem.removeSource(streamingSource)
+ uiTab.foreach(_.detach())
+ StreamingContext.setActiveContext(null)
+ waiter.notifyStop()
+ if (shutdownHookRef != null) {
+ shutdownHookRefToRemove = shutdownHookRef
+ shutdownHookRef = null
+ }
+ logInfo("StreamingContext stopped successfully")
+ }
+ // Even if we have already stopped, we still need to attempt to stop the SparkContext
+ // because a user might stop(stopSparkContext = false) and then call
+ // stop(stopSparkContext = true).
+ if (stopSparkContext) sc.stop()
--- End diff --
It is a different shutdown hook, but doesnt it end up calling SparkShutdownHookManager.remove() which locks the same lock that is causing this deadlock?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/9116
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148321658
Merged build finished. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42001035
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
--- End diff --
Here I want to set `shutdownHookRef` to `null`. To avoid the `check and remove` race condition, I added `var`.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/9116#discussion_r42080427
--- Diff: streaming/src/main/scala/org/apache/spark/streaming/StreamingContext.scala ---
@@ -694,31 +694,39 @@ class StreamingContext private[streaming] (
* @param stopGracefully if true, stops gracefully by waiting for the processing of all
* received data to be completed
*/
- def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = synchronized {
- try {
- state match {
- case INITIALIZED =>
- logWarning("StreamingContext has not been started yet")
- case STOPPED =>
- logWarning("StreamingContext has already been stopped")
- case ACTIVE =>
- scheduler.stop(stopGracefully)
- // Removing the streamingSource to de-register the metrics on stop()
- env.metricsSystem.removeSource(streamingSource)
- uiTab.foreach(_.detach())
- StreamingContext.setActiveContext(null)
- waiter.notifyStop()
- if (shutdownHookRef != null) {
- ShutdownHookManager.removeShutdownHook(shutdownHookRef)
- }
- logInfo("StreamingContext stopped successfully")
+ def stop(stopSparkContext: Boolean, stopGracefully: Boolean): Unit = {
+ var shutdownHookRefToRemove: AnyRef = null
+ synchronized {
+ try {
+ state match {
+ case INITIALIZED =>
+ logWarning("StreamingContext has not been started yet")
+ case STOPPED =>
+ logWarning("StreamingContext has already been stopped")
+ case ACTIVE =>
+ scheduler.stop(stopGracefully)
+ // Removing the streamingSource to de-register the metrics on stop()
+ env.metricsSystem.removeSource(streamingSource)
+ uiTab.foreach(_.detach())
+ StreamingContext.setActiveContext(null)
+ waiter.notifyStop()
+ if (shutdownHookRef != null) {
+ shutdownHookRefToRemove = shutdownHookRef
+ shutdownHookRef = null
+ }
+ logInfo("StreamingContext stopped successfully")
+ }
+ // Even if we have already stopped, we still need to attempt to stop the SparkContext
+ // because a user might stop(stopSparkContext = false) and then call
+ // stop(stopSparkContext = true).
+ if (stopSparkContext) sc.stop()
--- End diff --
it looks like this would call ShutdownHookManager.removeShutdownHook?
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/SparkContext.scala#L1703
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-11104][Streaming]Fix a deadlock in Stre...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9116#issuecomment-148359930
[Test build #43786 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43786/consoleFull) for PR 9116 at commit [`8060321`](https://github.com/apache/spark/commit/806032197ebd7494dd567482778174cf74be2645).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org