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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

GitHub user srowen opened a pull request:

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

    SPARK-1789. Multiple versions of Netty dependencies cause FlumeStreamSuite failure

    TL;DR is there is a bit of JAR hell trouble with Netty, that can be mostly resolved and will resolve a test failure.
    
    
    I hit the error described at http://apache-spark-user-list.1001560.n3.nabble.com/SparkContext-startup-time-out-td1753.html while running FlumeStreamingSuite, and have for a short while (is it just me?)
    
    velvia notes:
    "I have found a workaround.  If you add akka 2.2.4 to your dependencies, then everything works, probably because akka 2.2.4 brings in newer version of Jetty." 
    
    There are at least 3 versions of Netty in play in the build:
    
    - the new Flume 1.4.0 dependency brings in io.netty:netty:3.4.0.Final, and that is the immediate problem
    - the custom version of akka 2.2.3 depends on io.netty:netty:3.6.6.
    - but, Spark Core directly uses io.netty:netty-all:4.0.17.Final
    
    The POMs try to exclude other versions of netty, but are excluding org.jboss.netty:netty, when in fact older versions of io.netty:netty (not netty-all) are also an issue.
    
    The org.jboss.netty:netty excludes are largely unnecessary. I replaced many of them with io.netty:netty exclusions until everything agreed on io.netty:netty-all:4.0.17.Final.
    
    But this didn't work, since Akka 2.2.3 doesn't work with Netty 4.x. Down-grading to 3.6.6.Final across the board made some Spark code not compile.
    
    If the build *keeps* io.netty:netty:3.6.6.Final as well, everything seems to work. Part of the reason seems to be that Netty 3.x used the old `org.jboss.netty` packages. This is less than ideal, but is no worse than the current situation. 
    
    
    So this PR resolves the issue and improves the JAR hell, even if it leaves the existing theoretical Netty 3-vs-4 conflict:
    
    - Remove org.jboss.netty excludes where possible, for clarity; they're not needed except with Hadoop artifacts
    - Add io.netty:netty excludes where needed -- except, let akka keep its io.netty:netty
    - Change a bit of test code that actually depended on Netty 3.x, to use 4.x equivalent
    - Update SBT build accordingly
    
    A better change would be to update Akka far enough such that it agrees on Netty 4.x, but I don't know if that's feasible.


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

    $ git pull https://github.com/srowen/spark SPARK-1789

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

    https://github.com/apache/spark/pull/723.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 #723
    
----
commit 43661b75709896ccf88e79903ec15c9289172942
Author: Sean Owen <so...@cloudera.com>
Date:   2014-05-10T20:01:43Z

    Update and add Netty excludes to prevent some JAR conflicts that cause test issues

----


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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

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


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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#discussion_r12507254
  
    --- Diff: core/src/test/scala/org/apache/spark/LocalSparkContext.scala ---
    @@ -17,8 +17,7 @@
     
     package org.apache.spark
     
    -import org.jboss.netty.logging.InternalLoggerFactory
    -import org.jboss.netty.logging.Slf4JLoggerFactory
    +import _root_.io.netty.util.internal.logging.{Slf4JLoggerFactory, InternalLoggerFactory}
    --- End diff --
    
    Why is this necessary? Could this path be interpreted in some other way if `_root_` were ommited?


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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#issuecomment-42761247
  
    @srowen Okay looks good - I'll merge 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#issuecomment-42761070
  
    Hey @srowen - thanks for looking into this? Minor question, but otherwise looks good to 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.
---

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#discussion_r12507895
  
    --- Diff: core/src/test/scala/org/apache/spark/LocalSparkContext.scala ---
    @@ -17,8 +17,7 @@
     
     package org.apache.spark
     
    -import org.jboss.netty.logging.InternalLoggerFactory
    -import org.jboss.netty.logging.Slf4JLoggerFactory
    +import _root_.io.netty.util.internal.logging.{Slf4JLoggerFactory, InternalLoggerFactory}
    --- End diff --
    
    Yes you're right. I had thought it was conflicting with `scala.io` somehow but according to my IDE it's the Spark subpackage. (The abundance of import syntax in Scala is not my favorite thing.)


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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#discussion_r12507298
  
    --- Diff: core/src/test/scala/org/apache/spark/LocalSparkContext.scala ---
    @@ -17,8 +17,7 @@
     
     package org.apache.spark
     
    -import org.jboss.netty.logging.InternalLoggerFactory
    -import org.jboss.netty.logging.Slf4JLoggerFactory
    +import _root_.io.netty.util.internal.logging.{Slf4JLoggerFactory, InternalLoggerFactory}
    --- End diff --
    
    Ah I see. It collides with org.apache.spark.io because of the way Scala's relative imports work. I'm surprised we've never hit this before in Spark.


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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#issuecomment-42753816
  
    Merged build finished. All automated tests 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.
---

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#issuecomment-42753817
  
    All automated tests passed.
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14872/


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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#issuecomment-42752877
  
    Merged build started. 


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

[GitHub] spark pull request: SPARK-1789. Multiple versions of Netty depende...

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

    https://github.com/apache/spark/pull/723#issuecomment-42752876
  
     Merged build triggered. 


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