You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by tsliwowicz <gi...@git.apache.org> on 2014/10/22 01:53:17 UTC

[GitHub] spark pull request: [SPARK-4006] In long running contexts, we enco...

GitHub user tsliwowicz opened a pull request:

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

    [SPARK-4006] In long running contexts, we encountered the situation of double registe...

    ...r without a remove in between. The cause for that is unknown, and assumed a temp network issue.
    
    However, since the second register is with a BlockManagerId on a different port, blockManagerInfo.contains() returns false, while blockManagerIdByExecutor returns Some. This inconsistency is caught in a conditional statement that does System.exit(1), which is a huge robustness issue for us.
    
    The fix - simply remove the old id from both maps during register when this happens. We are mimicking the behavior of expireDeadHosts(), by doing local cleanup of the maps before trying to add new ones.
    
    Also - added some logging for register and unregister.
    
    This is just like https://github.com/apache/spark/pull/2854 except it's on master

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

    $ git pull https://github.com/taboola/spark master-block-mgr-removal

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

    https://github.com/apache/spark/pull/2886.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 #2886
    
----
commit f48bce9cc25fa2672ea36bd90e64854159de8ead
Author: Tal Sliwowicz <ta...@taboola.com>
Date:   2014-10-21T14:29:39Z

    In long running contexts, we encountered the situation of double register without a remove in between. The cause for that is unknown, and assumed a temp network issue.
    
        However, since the second register is with a BlockManagerId on a different port, blockManagerInfo.contains() returns false, while blockManagerIdByExecutor returns Some. This inconsistency is caught in a conditional statement that does System.exit(1), which is a huge robustness issue for us.
    
        The fix - simply remove the old id from both maps during register when this happens. We are mimicking the behavior of expireDeadHosts(), by doing local cleanup of the maps before trying to add new ones.
    
        Also - added some logging for register and unregister.

----


---
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-4006] In long running contexts, we enco...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60313702
  
    Thanks a lot @tsliwowicz I'll do that shortly.


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#discussion_r19294252
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManagerMasterActor.scala ---
    @@ -325,22 +325,23 @@ class BlockManagerMasterActor(val isLocal: Boolean, conf: SparkConf, listenerBus
     
       private def register(id: BlockManagerId, maxMemSize: Long, slaveActor: ActorRef) {
         val time = System.currentTimeMillis()
    +    
         if (!blockManagerInfo.contains(id)) {
           blockManagerIdByExecutor.get(id.executorId) match {
             case Some(manager) =>
    -          // A block manager of the same executor already exists.
    -          // This should never happen. Let's just quit.
    -          logError("Got two different block manager registrations on " + id.executorId)
    -          System.exit(1)
    +          // A block manager of the same executor already exists so remove it (assumed dead).
    --- End diff --
    
    actually what I meant was to add a comma between "exists" and "so"... It's ok I can fix this myself when I merge 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 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#discussion_r19250038
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManagerMasterActor.scala ---
    @@ -325,22 +325,23 @@ class BlockManagerMasterActor(val isLocal: Boolean, conf: SparkConf, listenerBus
     
       private def register(id: BlockManagerId, maxMemSize: Long, slaveActor: ActorRef) {
         val time = System.currentTimeMillis()
    +    
         if (!blockManagerInfo.contains(id)) {
           blockManagerIdByExecutor.get(id.executorId) match {
             case Some(manager) =>
    -          // A block manager of the same executor already exists.
    -          // This should never happen. Let's just quit.
    -          logError("Got two different block manager registrations on " + id.executorId)
    -          System.exit(1)
    +          // A block manager of the same executor already exists so remove it (assumed dead).
    +          logError("Got two different block manager registrations on same executor - " 
    +              + " will remove, new Id " + id + ", orig id - " + manager)
    +          removeExecutor(id.executorId)  
             case None =>
    -          blockManagerIdByExecutor(id.executorId) = id
           }
    -
    -      logInfo("Registering block manager %s with %s RAM".format(
    -        id.hostPort, Utils.bytesToString(maxMemSize)))
    -
    -      blockManagerInfo(id) =
    -        new BlockManagerInfo(id, time, maxMemSize, slaveActor)
    +      logInfo("Registering block manager %s with %s RAM, %s".format(
    +        id.hostPort, Utils.bytesToString(maxMemSize), id))
    +      
    +      blockManagerIdByExecutor(id.executorId) = id
    +      
    +      blockManagerInfo(id) = new BlockManagerInfo(id, System.currentTimeMillis(), 
    +                                                  maxMemSize, slaveActor)
    --- End diff --
    
    Can you use this format
    ```
    blockManagerInfo(id) = new BlockManagerInfo(
      id, System.currentTimeMillis, maxMemSize, slaveActor)
    ```


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60228510
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22072/consoleFull) for   PR 2886 at commit [`094d508`](https://github.com/apache/spark/commit/094d508fed9aa57beb60d7a571cbe7c1e3b334c1).
     * 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60221739
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22072/consoleFull) for   PR 2886 at commit [`094d508`](https://github.com/apache/spark/commit/094d508fed9aa57beb60d7a571cbe7c1e3b334c1).
     * 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 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60227762
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22070/
    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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#discussion_r19249800
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManagerMasterActor.scala ---
    @@ -325,22 +325,23 @@ class BlockManagerMasterActor(val isLocal: Boolean, conf: SparkConf, listenerBus
     
       private def register(id: BlockManagerId, maxMemSize: Long, slaveActor: ActorRef) {
         val time = System.currentTimeMillis()
    +    
         if (!blockManagerInfo.contains(id)) {
           blockManagerIdByExecutor.get(id.executorId) match {
             case Some(manager) =>
    -          // A block manager of the same executor already exists.
    -          // This should never happen. Let's just quit.
    -          logError("Got two different block manager registrations on " + id.executorId)
    -          System.exit(1)
    +          // A block manager of the same executor already exists so remove it (assumed dead).
    +          logError("Got two different block manager registrations on same executor - " 
    +              + " will remove, new Id " + id + ", orig id - " + manager)
    --- End diff --
    
    Could you write this in string interpolation or formatting so it's easier to read
    e.g. `s"new ID $id"` or `"new ID %s".format(id)"`


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60228517
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22072/
    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-4006] In long running contexts, we enco...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60019018
  
    ok to test


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60227754
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22070/consoleFull) for   PR 2886 at commit [`df9d98f`](https://github.com/apache/spark/commit/df9d98fe6703f6cc37fb0187fa55d140f37bb50e).
     * 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-4006] In long running contexts, we enco...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60168995
  
    Hey @tsliwowicz there are a few style and wording issues that I'd like to see fixed. Also, I would prefer to have the whitespace changes reverted. However, I think the fix is correct and this LGTM logically.


---
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-4006] In long running contexts, we enco...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60279888
  
    Ok I have merged this into master. It doesn't merge cleanly into 1.1, so @tsliwowicz can you create a new PR against that branch? 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 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60220879
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22070/consoleFull) for   PR 2886 at commit [`df9d98f`](https://github.com/apache/spark/commit/df9d98fe6703f6cc37fb0187fa55d140f37bb50e).
     * 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 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-4006] In long running contexts, we enco...

Posted by tsliwowicz <gi...@git.apache.org>.
Github user tsliwowicz commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60221362
  
    @andrewor14  - thanks for the comments. I believe I fixed them all. Let me know!


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60024273
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22016/consoleFull) for   PR 2886 at commit [`f48bce9`](https://github.com/apache/spark/commit/f48bce9cc25fa2672ea36bd90e64854159de8ead).
     * 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60019511
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22016/consoleFull) for   PR 2886 at commit [`f48bce9`](https://github.com/apache/spark/commit/f48bce9cc25fa2672ea36bd90e64854159de8ead).
     * 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 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#discussion_r19250102
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManagerMasterActor.scala ---
    @@ -203,6 +202,7 @@ class BlockManagerMasterActor(val isLocal: Boolean, conf: SparkConf, listenerBus
           }
         }
         listenerBus.post(SparkListenerBlockManagerRemoved(System.currentTimeMillis(), blockManagerId))
    +    logInfo("removed " + blockManagerId)
    --- End diff --
    
    Can you make this a little more formal logInfo(s"Removing block manager $blockManagerId")


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60222452
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22071/
    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 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#discussion_r19249967
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManagerMasterActor.scala ---
    @@ -325,22 +325,23 @@ class BlockManagerMasterActor(val isLocal: Boolean, conf: SparkConf, listenerBus
     
       private def register(id: BlockManagerId, maxMemSize: Long, slaveActor: ActorRef) {
         val time = System.currentTimeMillis()
    +    
         if (!blockManagerInfo.contains(id)) {
           blockManagerIdByExecutor.get(id.executorId) match {
             case Some(manager) =>
    -          // A block manager of the same executor already exists.
    -          // This should never happen. Let's just quit.
    -          logError("Got two different block manager registrations on " + id.executorId)
    -          System.exit(1)
    +          // A block manager of the same executor already exists so remove it (assumed dead).
    +          logError("Got two different block manager registrations on same executor - " 
    +              + " will remove, new Id " + id + ", orig id - " + manager)
    --- End diff --
    
    Also, the message is confusing. How about
    ```
    logError(s"Received two different block manager registrations on the same executor - will replace the old one $oldId with the new $newId")
    ```
    Assuming you rename `manager` to `oldId` or something.


---
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-4006] In long running contexts, we enco...

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

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


---
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-4006] In long running contexts, we enco...

Posted by tsliwowicz <gi...@git.apache.org>.
Github user tsliwowicz commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60222733
  
    the failure seems technical (not related to my fix), I think. Local maven build works fine for me.


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60024276
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22016/
    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-4006] In long running contexts, we enco...

Posted by tsliwowicz <gi...@git.apache.org>.
Github user tsliwowicz commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60312692
  
    @andrewor14 I created PR for 1.0, 1.1 and updated the 0.9 PR - can you please review and merge if 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-4006] In long running contexts, we enco...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60280998
  
    (also branch 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 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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#discussion_r19249767
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManagerMasterActor.scala ---
    @@ -325,22 +325,23 @@ class BlockManagerMasterActor(val isLocal: Boolean, conf: SparkConf, listenerBus
     
       private def register(id: BlockManagerId, maxMemSize: Long, slaveActor: ActorRef) {
         val time = System.currentTimeMillis()
    +    
         if (!blockManagerInfo.contains(id)) {
           blockManagerIdByExecutor.get(id.executorId) match {
             case Some(manager) =>
    -          // A block manager of the same executor already exists.
    -          // This should never happen. Let's just quit.
    -          logError("Got two different block manager registrations on " + id.executorId)
    -          System.exit(1)
    +          // A block manager of the same executor already exists so remove it (assumed dead).
    --- End diff --
    
    already exists, so remove 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 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-4006] In long running contexts, we enco...

Posted by tsliwowicz <gi...@git.apache.org>.
Github user tsliwowicz commented on the pull request:

    https://github.com/apache/spark/pull/2886#issuecomment-60306506
  
    will do. Can you also merge into the 0.9 branch? I will update the PR I already have for it. https://github.com/apache/spark/pull/2854


---
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-4006] In long running contexts, we enco...

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

    https://github.com/apache/spark/pull/2886#issuecomment-60018087
  
    Can one of the admins verify this patch?


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