You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ssaavedra <gi...@git.apache.org> on 2017/10/04 11:12:42 UTC

[GitHub] spark pull request #19427: Reset spark.driver.bindAddress when starting a Ch...

GitHub user ssaavedra opened a pull request:

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

    Reset spark.driver.bindAddress when starting a Checkpoint

    ## What changes were proposed in this pull request?
    
    It seems that recovering from a checkpoint can replace the old
    driver and executor IP addresses, as the workload can now be taking
    place in a different cluster configuration. It follows that the
    bindAddress for the master may also have changed. Thus we should not be
    keeping the old one, and instead be added to the list of properties to
    reset and recreate from the new environment.
    
    ## How was this patch tested?
    
    This patch was tested via manual testing on AWS, using the experimental (not yet merged) Kubernetes scheduler, which uses bindAddress to bind to a Kubernetes service (and thus was how I first encountered the bug too), but it is not a code-path related to the scheduler and this may have slipped through when merging SPARK-4563.

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

    $ git pull https://github.com/ssaavedra/spark fix-checkpointing-master

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

    https://github.com/apache/spark/pull/19427.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 #19427
    
----
commit 892555f452173b73aeabc077749c4c32a7d4e504
Author: Santiago Saavedra <ss...@openshine.com>
Date:   2017-09-28T15:30:29Z

    Reset spark.driver.bindAddress when starting a Checkpoint

----


---

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


[GitHub] spark issue #19427: [SparkStreaming] Reset spark.driver.bindAddress when sta...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    yes, if you can open an issue in JIRA and update this PR title it should link automatically.


---

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


[GitHub] spark pull request #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddr...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19427#discussion_r148621269
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/Checkpoint.scala ---
    @@ -62,6 +63,7 @@ class Checkpoint(ssc: StreamingContext, val checkpointTime: Time)
     
         val newSparkConf = new SparkConf(loadDefaults = false).setAll(sparkConfPairs)
           .remove("spark.driver.host")
    +      .remove("spark.driver.bindAddress")
    --- End diff --
    
    Do we have to remove this? It means we must drop `spark.driver.bindAddress` if it's not set in the new run.


---

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


[GitHub] spark issue #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddress whe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    **[Test build #82908 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82908/testReport)** for PR 19427 at commit [`892555f`](https://github.com/apache/spark/commit/892555f452173b73aeabc077749c4c32a7d4e504).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddress whe...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    **[Test build #82908 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82908/testReport)** for PR 19427 at commit [`892555f`](https://github.com/apache/spark/commit/892555f452173b73aeabc077749c4c32a7d4e504).


---

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


[GitHub] spark pull request #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddr...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

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


[GitHub] spark issue #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddress whe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82908/
    Test PASSed.


---

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


[GitHub] spark issue #19427: Reset spark.driver.bindAddress when starting a Checkpoin...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    @ssaavedra Could you also update the Title as [SPARK-XXXXX][component] Title... please.


---

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


[GitHub] spark issue #19427: [SparkStreaming] Reset spark.driver.bindAddress when sta...

Posted by yssharma <gi...@git.apache.org>.
Github user yssharma commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    I don't think there is an automated way. You could create a JIRA ticket  and rename this title with the ticket id and component name.


---

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


[GitHub] spark issue #19427: Reset spark.driver.bindAddress when starting a Checkpoin...

Posted by ssaavedra <gi...@git.apache.org>.
Github user ssaavedra commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    Should I create the appropriate issue in JIRA? I'm not sure if there is any automation which does that.


---

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


[GitHub] spark issue #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddress whe...

Posted by ssaavedra <gi...@git.apache.org>.
Github user ssaavedra commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    Is anyone considering this patch? Should I advertise it anywhere else?


---

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


[GitHub] spark issue #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddress whe...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:

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


---

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


[GitHub] spark issue #19427: Reset spark.driver.bindAddress when starting a Checkpoin...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark pull request #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddr...

Posted by ssaavedra <gi...@git.apache.org>.
Github user ssaavedra commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19427#discussion_r148945009
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/Checkpoint.scala ---
    @@ -62,6 +63,7 @@ class Checkpoint(ssc: StreamingContext, val checkpointTime: Time)
     
         val newSparkConf = new SparkConf(loadDefaults = false).setAll(sparkConfPairs)
           .remove("spark.driver.host")
    +      .remove("spark.driver.bindAddress")
    --- End diff --
    
    Yes. If it is not set in the new run, it should still be meaningless anyway. It makes sense to know this property on the subsequent calls to spark-submit. If we are resuming a checkpoint it means we are re-submitting work, but it may be run in a different cluster configuration, and thus we may want to change the bindAddress or this different configuration may even wish to rely on falling back to the `spark.driver.host` configuration. In any case, it should make no sense to keep the old setting, unless we are running on a static configuration, in which case it is not a caveat to remove this, as the command-line to re-launch the job can still re-populate the property if it needs to keep being the same.


---

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


[GitHub] spark issue #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddress whe...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    Jenkins, test this please


---

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


[GitHub] spark issue #19427: [SPARK-22294][Deploy] Reset spark.driver.bindAddress whe...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19427
  
    Merged build finished. Test PASSed.


---

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