[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-10-25 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
Thank you.



---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-10-24 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
@srowen Can you have another look?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #21816: [SPARK-24794][CORE] Driver launched through rest ...

2018-10-16 Thread bsikander
Github user bsikander commented on a diff in the pull request:

https://github.com/apache/spark/pull/21816#discussion_r225553817
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/rest/StandaloneRestSubmitSuite.scala
 ---
@@ -83,6 +83,26 @@ class StandaloneRestSubmitSuite extends SparkFunSuite 
with BeforeAndAfterEach {
 assert(submitResponse.success)
   }
 
+  test("create submission with multiple masters") {
+val submittedDriverId = "your-driver-id"
+val submitMessage = "my driver is submitted"
+val masterUrl = startDummyServer(submitId = submittedDriverId, 
submitMessage = submitMessage)
+val conf = new SparkConf(loadDefaults = false)
+val RANDOM_PORT = 9000
+val allMasters = "%s,%s:%d".format(masterUrl, Utils.localHostName(), 
RANDOM_PORT)
--- End diff --

@srowen I was away for a few days. Will respond back soon.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-10-04 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
Failing tests don't seem to be related to this change.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-10-02 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
>> So is there no behavior change if your master specifies only one master?
If user specifies only 1 master then there is no behavioral change.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-10-01 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
@srowen I removed the extra .toString.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-10-01 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
@srowen even though it is unlikely but it could happen. I have updated the 
code.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-09-26 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
>> What might break if we make this change? I think that's what any other 
reviewer cares abou
To the best of my knowledge. Nothing should break. 


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #21816: [SPARK-24794][CORE] Driver launched through rest ...

2018-09-26 Thread bsikander
Github user bsikander commented on a diff in the pull request:

https://github.com/apache/spark/pull/21816#discussion_r220119995
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/rest/StandaloneRestServer.scala ---
@@ -138,6 +138,10 @@ private[rest] class StandaloneSubmitRequestServlet(
 val driverExtraClassPath = 
sparkProperties.get("spark.driver.extraClassPath")
 val driverExtraLibraryPath = 
sparkProperties.get("spark.driver.extraLibraryPath")
 val superviseDriver = sparkProperties.get("spark.driver.supervise")
+val masters = sparkProperties.get("spark.master")
--- End diff --

I have added the comment.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #21816: [SPARK-24794][CORE] Driver launched through rest ...

2018-09-26 Thread bsikander
Github user bsikander commented on a diff in the pull request:

https://github.com/apache/spark/pull/21816#discussion_r220120052
  
--- Diff: 
core/src/main/scala/org/apache/spark/deploy/rest/StandaloneRestServer.scala ---
@@ -138,6 +138,10 @@ private[rest] class StandaloneSubmitRequestServlet(
 val driverExtraClassPath = 
sparkProperties.get("spark.driver.extraClassPath")
 val driverExtraLibraryPath = 
sparkProperties.get("spark.driver.extraLibraryPath")
 val superviseDriver = sparkProperties.get("spark.driver.supervise")
+val masters = sparkProperties.get("spark.master")
+val masterPort = 
Utils.extractHostPortFromSparkUrl(masterUrl)._2.toString
--- End diff --

Done.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-09-24 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
@vanzin ok, how can I find the right reviewer for this change? Posting on 
dev mailing list?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-09-22 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
Could some please have a look at this?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-09-08 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
could someone please have a look at this change?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-08-26 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
Sorry for dropping message again, but could someone please have a look at 
this change. This is a problem that I am having constantly and a validation 
from senior committers will be really helpful.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-08-16 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
@holdenk somehow, Jenkins didn't execute the tests.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark issue #21816: [SPARK-24794][CORE] Driver launched through rest should ...

2018-07-27 Thread bsikander
Github user bsikander commented on the issue:

https://github.com/apache/spark/pull/21816
  
@vanzin Could you please have a look on this change?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #21816: [SPARK-24794][CORE] Driver launched through rest ...

2018-07-19 Thread bsikander
GitHub user bsikander opened a pull request:

https://github.com/apache/spark/pull/21816

[SPARK-24794][CORE] Driver launched through rest should use all masters

## What changes were proposed in this pull request?

In standalone cluster mode, one could launch driver with supervise mode
enabled. StandaloneRestServer class uses the host and port of current
master as the spark.master property while launching the driver
(even if you are running in HA mode). This class also ignores the
spark.master property passed as part of the request.

Due to the above problem, if the Spark masters switch due to some reason
and your driver is killed unexpectedly and relaunched, it will try to
connect to the master which is in the driver command specified as
-Dspark.master. But this master will be in STANDBY mode and after trying
multiple times, the SparkContext will kill itself (even though secondary
master was alive and healthy).

This change picks the spark.master property from request and uses it to
launch the driver process. Due to this, the driver process has both
masters in -Dspark.master property. Even if the masters switch, SparkContext
can still connect to the ALIVE master and work correctly.

## How was this patch tested?
This patch was manually tested on a standalone cluster running 2.2.1. It 
was rebased on current master and all tests were executed. I have added a unit 
test for this change (but since I am new I hope I have covered all).


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/bsikander/spark rest_driver_fix

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/21816.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 #21816


commit 1f13cd195fc1d9f21f6bf2808032b8a31c46736e
Author: Behroz Sikander 
Date:   2018-07-17T12:58:33Z

[SPARK-24794][CORE] Driver launched through rest should use all masters

In standalone cluster mode, one could launch driver with supervise mode
enabled. StandaloneRestServer class uses the host and port of current
master as the spark.master property while launching the driver
(even if you are running in HA mode). This class also ignores the
spark.master property passed as part of the request.

Due to the above problem, if the Spark masters switch due to some reason
and your driver is killed unexpectedly and relaunched, it will try to
connect to the master which is in the driver command specified as
-Dspark.master. But this master will be in STANDBY mode and after trying
multiple times, the SparkContext will kill itself (even though secondary
master was alive and healthy).

This change picks the spark.master property from request and uses it to
launch the driver process. Due to this, the driver process has both
masters in -Dspark.master property. Even if the masters switch, SparkContext
can still connect to the ALIVE master and work correctly.




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org