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

Posted to reviews@spark.apache.org by Victsm <gi...@git.apache.org> on 2017/11/29 03:20:06 UTC

[GitHub] spark pull request #19839: SPARK-22373 Bump Janino dependency version to fix...

GitHub user Victsm opened a pull request:

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

    SPARK-22373 Bump Janino dependency version to fix thread safety issue…

    … with Janino when compiling generated code.
    
    ## What changes were proposed in this pull request?
    
    Bump up Janino dependency version to fix thread safety issue during compiling generated code
    
    ## How was this patch tested?
    
    Check https://issues.apache.org/jira/browse/SPARK-22373 for details.
    Converted part of the code in CodeGenerator into a standalone application, so the issue can be consistently reproduced locally.
    Verified that changing Janino dependency version resolved this issue.


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

    $ git pull https://github.com/Victsm/spark SPARK-22373

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

    https://github.com/apache/spark/pull/19839.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 #19839
    
----
commit 5f54a89fddacad5f041fb35e46b7faba27c4789d
Author: Min Shen <ms...@linkedin.com>
Date:   2017-11-29T03:14:51Z

    SPARK-22373 Bump Janino dependency version to fix thread safety issue with Janino when compiling generated code.

----


---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    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 #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    Which version? if it's unreleased, I don't see a harm in updating this now and later too. If it's released, we can just update it here.


---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user Victsm commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    @srowen @kiszk @mgaido91 
    
    Is this patch ready to merge?



---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    @srowen, seems this one causes a build failure for branch-2.1:
    
    https://amplab.cs.berkeley.edu/jenkins/job/spark-branch-2.1-test-sbt-hadoop-2.2/641/console
    https://amplab.cs.berkeley.edu/jenkins/job/spark-branch-2.1-test-sbt-hadoop-2.3/634/console
    https://amplab.cs.berkeley.edu/jenkins/job/spark-branch-2.1-test-sbt-hadoop-2.4/627/console
    https://amplab.cs.berkeley.edu/jenkins/job/spark-branch-2.1-test-sbt-hadoop-2.6/646/console
    
    ```
    To update the manifest file, run './dev/test-dependencies.sh --replace-manifest'.
    diff --git a/dev/deps/spark-deps-hadoop-2.2 b/dev/pr-deps/spark-deps-hadoop-2.2
    index 635b3a6..5940601 100644
    --- a/dev/deps/spark-deps-hadoop-2.2
    +++ b/dev/pr-deps/spark-deps-hadoop-2.2
    @@ -24,7 +24,7 @@ commons-beanutils-core-1.8.0.jar
     commons-cli-1.2.jar
     commons-codec-1.10.jar
     commons-collections-3.2.2.jar
    -commons-compiler-3.0.0.jar
    +commons-compiler-3.0.7.jar
     commons-compress-1.4.1.jar
     commons-configuration-1.6.jar
     commons-crypto-1.0.0.jar
    @@ -80,7 +80,7 @@ jackson-databind-2.6.5.jar
     jackson-mapper-asl-1.9.13.jar
     jackson-module-paranamer-2.6.5.jar
     jackson-module-scala_2.11-2.6.5.jar
    -janino-3.0.0.jar
    +janino-3.0.7.jar
     javassist-3.18.1-GA.jar
     javax.annotation-api-1.2.jar
     javax.inject-1.jar
    ```
    
    Seems it's because `./dev/deps/spark-deps-hadoop-2.2`, `./dev/deps/spark-deps-hadoop-2.3` and`./dev/deps/spark-deps-hadoop-2.4` should be updated too in branch-2.1.


---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    Yea, we would like to use a version that includes [this](https://github.com/janino-compiler/janino/issues/33). However, it is not released yet.
    
    SGTM to use the latest released version after running a script.


---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user mgaido91 commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    No, it is unreleased. Yes, sure, we can also update it now and later too.


---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    **[Test build #84322 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84322/testReport)** for PR 19839 at commit [`ede95ae`](https://github.com/apache/spark/commit/ede95aec6bd2575b4a576178df299fa5b656a9ec).


---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user mgaido91 commented on the issue:

    https://github.com/apache/spark/pull/19839
  
    I think we don't need this since we need to upgrade to the next janino release for the issue related to SPARK-18016.


---

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

[GitHub] spark issue #19839: SPARK-22373 Bump Janino dependency version to fix thread...

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

Github user AmplabJenkins commented on the issue:

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