You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by tedyu <gi...@git.apache.org> on 2015/10/23 18:25:44 UTC

[GitHub] spark pull request: Make Outbox stopped exception singleton

GitHub user tedyu opened a pull request:

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

    Make Outbox stopped exception singleton

    

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

    $ git pull https://github.com/tedyu/spark master

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

    https://github.com/apache/spark/pull/9254.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 #9254
    
----
commit 79c6e00dae89de815de73e9ce66f47d9e0cb6bdd
Author: tedyu <yu...@gmail.com>
Date:   2015-10-23T16:25:15Z

    Make Outbox stopped exception singleton

----


---
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-11286 Make Outbox stopped exception sing...

Posted by vanzin <gi...@git.apache.org>.

Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9254#discussion_r42896850
  
    --- Diff: core/src/main/scala/org/apache/spark/rpc/netty/Outbox.scala ---
    @@ -215,7 +216,7 @@ private[netty] class Outbox(nettyEnv: NettyRpcEnv, val address: RpcAddress) {
         // update messages and it's safe to just drain the queue.
         var message = messages.poll()
    --- End diff --
    
    I doubt there will be many messages in the outbox at a time. This code mostly fixes a race during initialization of the RPC channel, later the outbox would be mostly a bypass. #9210 even bypasses the outbox altogether in a lot of cases.


---
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-11286 Make Outbox stopped exception sing...

Posted by tedyu <gi...@git.apache.org>.

Github user tedyu commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9254#discussion_r42896202
  
    --- Diff: core/src/main/scala/org/apache/spark/rpc/netty/Outbox.scala ---
    @@ -215,7 +216,7 @@ private[netty] class Outbox(nettyEnv: NettyRpcEnv, val address: RpcAddress) {
         // update messages and it's safe to just drain the queue.
         var message = messages.poll()
    --- End diff --
    
    In that case the exception is no longer a singleton - it is created inside handleNetworkFailure().
    Network failure should be rare but when it happens, there may be non-trivial number of messages to be notified.


---
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-11286 Make Outbox stopped exception sing...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/9254#issuecomment-150658341
  
    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-11286 Make Outbox stopped exception sing...

Posted by vanzin <gi...@git.apache.org>.

Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9254#discussion_r42895844
  
    --- Diff: core/src/main/scala/org/apache/spark/rpc/netty/Outbox.scala ---
    @@ -215,7 +216,7 @@ private[netty] class Outbox(nettyEnv: NettyRpcEnv, val address: RpcAddress) {
         // update messages and it's safe to just drain the queue.
         var message = messages.poll()
    --- End diff --
    
    But why? What's being optimized here? It's such a rare occurrence that it's better to pay the allocation cost when it happens than waste the memory to keep the singleton around.


---
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-11286 Make Outbox stopped exception sing...

Posted by tedyu <gi...@git.apache.org>.

Github user tedyu commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9254#discussion_r42898550
  
    --- Diff: core/src/main/scala/org/apache/spark/rpc/netty/Outbox.scala ---
    @@ -215,7 +216,7 @@ private[netty] class Outbox(nettyEnv: NettyRpcEnv, val address: RpcAddress) {
         // update messages and it's safe to just drain the queue.
         var message = messages.poll()
    --- End diff --
    
    Thanks for providing richer background information.


---
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-11286 Make Outbox stopped exception sing...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9254#issuecomment-150627414
  
    **[Test build #44230 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44230/consoleFull)** for PR 9254 at commit [`79c6e00`](https://github.com/apache/spark/commit/79c6e00dae89de815de73e9ce66f47d9e0cb6bdd).


---
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-11286 Make Outbox stopped exception sing...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/9254#issuecomment-150658153
  
    **[Test build #44230 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44230/consoleFull)** for PR 9254 at commit [`79c6e00`](https://github.com/apache/spark/commit/79c6e00dae89de815de73e9ce66f47d9e0cb6bdd).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:\n  * `    case class Data(feature: Int)`\n


---
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-11286 Make Outbox stopped exception sing...

Posted by tedyu <gi...@git.apache.org>.

Github user tedyu commented on a diff in the pull request:

    https://github.com/apache/spark/pull/9254#discussion_r42895452
  
    --- Diff: core/src/main/scala/org/apache/spark/rpc/netty/Outbox.scala ---
    @@ -215,7 +216,7 @@ private[netty] class Outbox(nettyEnv: NettyRpcEnv, val address: RpcAddress) {
         // update messages and it's safe to just drain the queue.
         var message = messages.poll()
    --- End diff --
    
    Exception creation can be done here instead of inside the loop, keeping usable stack trace


---
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-11286 Make Outbox stopped exception sing...

Posted by rxin <gi...@git.apache.org>.

Github user rxin commented on the pull request:

    https://github.com/apache/spark/pull/9254#issuecomment-150647468
  
    Ted - why is this change necessary? These only happen in error cases so I'm not sure how useful it is to optimize then. On top of that, this seems to change the stacktrace coming from the exceptions.



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