[GitHub] spark issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread zsxwing
Github user zsxwing commented on the issue:

https://github.com/apache/spark/pull/18126
  
Thanks! Merging to master and 2.2.


---
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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/18126
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77438/
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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/18126
  
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 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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/18126
  
**[Test build #77438 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77438/testReport)**
 for PR 18126 at commit 
[`ca2c9c5`](https://github.com/apache/spark/commit/ca2c9c53040373e82ff350c1b3c77c1512926cec).
 * 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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread BryanCutler
Github user BryanCutler commented on the issue:

https://github.com/apache/spark/pull/18126
  
No, I couldn't think of anything else without it being too long winded.  I
agree that the `worker` prefix gives enough meaning, plus whomever uses
this should already know the context that it's intended for.

On Fri, May 26, 2017 at 4:37 PM, Shixiong Zhu 
wrote:

> *@zsxwing* commented on this pull request.
> --
>
> In core/src/main/scala/org/apache/spark/deploy/worker/DriverRunner.scala
> :
>
> > @@ -57,7 +57,8 @@ private[deploy] class DriverRunner(
>@volatile private[worker] var finalException: Option[Exception] = None
>
>// Timeout to wait for when trying to terminate a driver.
> -  private val DRIVER_TERMINATE_TIMEOUT_MS = 10 * 1000
> +  private val DRIVER_TERMINATE_TIMEOUT_MS =
> +conf.getTimeAsMs("spark.worker.driverTerminateTimeout", "10s")
>
> spark.worker means this is only for Spark workers, so I think it should
> be obvious. Do you have a better config name?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> , or 
mute
> the 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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread zsxwing
Github user zsxwing commented on the issue:

https://github.com/apache/spark/pull/18126
  
> 10s is pretty short for a driver timeout

This is usually not a problem. If worker is trying to kill a driver, it 
often means the driver is unhealthy or being killed by the user intentionally. 
10 seconds to allow shutdown hooks cleaning up resources such as deleting temp 
files is usually enough. Given that replying on shutdown hooks to persist data 
is not common, and we have a configuration for special cases, I think it's fine.


---
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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread vanzin
Github user vanzin commented on the issue:

https://github.com/apache/spark/pull/18126
  
Ah, I thought the original change was in 2.2 only. Looks good then.


---
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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread zsxwing
Github user zsxwing commented on the issue:

https://github.com/apache/spark/pull/18126
  
This is the behavior in 2.1.0, if we change the default value to 
`Long.MaxValue`, it would surprise users again :(.

I'm inclined to keep it as 2.1.0.


---
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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread vanzin
Github user vanzin commented on the issue:

https://github.com/apache/spark/pull/18126
  
Looks ok.

I wonder if keeping the old behavior by default wouldn't be better, to 
avoid surprising users who upgrade and run into 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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread SparkQA
Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/18126
  
**[Test build #77438 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77438/testReport)**
 for PR 18126 at commit 
[`ca2c9c5`](https://github.com/apache/spark/commit/ca2c9c53040373e82ff350c1b3c77c1512926cec).


---
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 issue #18126: [SPARK-20843][Core]Add a config to set driver terminate ...

2017-05-26 Thread zsxwing
Github user zsxwing commented on the issue:

https://github.com/apache/spark/pull/18126
  
cc @vanzin @BryanCutler 


---
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