Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into from

Conversation

Victsm
Copy link
Contributor

@Victsm Victsm commented Nov 29, 2017

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

@mgaido91
Copy link
Contributor

I think we don't need this since we need to upgrade to the next janino release for the issue related to SPARK-18016.

@srowen
Copy link
Member

srowen commented Nov 29, 2017

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.

@SparkQA
Copy link

SparkQA commented Nov 29, 2017

Test build #3997 has finished for PR 19839 at commit 5f54a89.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Nov 29, 2017

You'll need to run ./dev/test-dependencies.sh --replace-manifest

@mgaido91
Copy link
Contributor

No, it is unreleased. Yes, sure, we can also update it now and later too.

@kiszk
Copy link
Member

kiszk commented Nov 29, 2017

Yea, we would like to use a version that includes this. However, it is not released yet.

SGTM to use the latest released version after running a script.

@HyukjinKwon
Copy link
Member

test this please

@SparkQA
Copy link

SparkQA commented Nov 30, 2017

Test build #84322 has finished for PR 19839 at commit ede95ae.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@Victsm
Copy link
Contributor Author

Victsm commented Dec 1, 2017

@srowen @kiszk @mgaido91

Is this patch ready to merge?

asfgit pushed a commit that referenced this pull request Dec 1, 2017
… 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.

Author: Min Shen <mshen@linkedin.com>

Closes #19839 from Victsm/SPARK-22373.

(cherry picked from commit 7da1f57)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@srowen
Copy link
Member

srowen commented Dec 1, 2017

Merged to master/2.2/2.1

@asfgit asfgit closed this in 7da1f57 Dec 1, 2017
asfgit pushed a commit that referenced this pull request Dec 1, 2017
… 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.

Author: Min Shen <mshen@linkedin.com>

Closes #19839 from Victsm/SPARK-22373.

(cherry picked from commit 7da1f57)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@HyukjinKwon
Copy link
Member

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

@HyukjinKwon
Copy link
Member

Will actually open a PR shortly :).

MatthewRBruce pushed a commit to Shopify/spark that referenced this pull request Jul 31, 2018
… 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.

Author: Min Shen <mshen@linkedin.com>

Closes apache#19839 from Victsm/SPARK-22373.

(cherry picked from commit 7da1f57)
Signed-off-by: Sean Owen <sowen@cloudera.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants