-
Notifications
You must be signed in to change notification settings - Fork 211
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
Kindly ask to maintain 3.0.x bugfix versions in parallel (or one time release for 3.0.16) #115
Comments
Do you want a release 3.0.16? |
Yeah please. It would be ideal if you don't mind to release both 3.0.16 and 3.1.2 (as I think my patches addressed all Spark test failures from 3.1.1) so that I could ask Spark community to pick one. Spark community also noticed there's stack overflow error in 3.0.x (https://issues.apache.org/jira/browse/SPARK-25987) and I figured out the reason as how flowAnalysis works. Spark generates the code containing lots of branches, which triggers "recursive call" even they're not nested. So it's a good news Janino 3.1.x replaced it with stack map (I hadn't imagine about the bug) - if Spark community feel good about stability on 3.1.x, Spark community would love to pick 3.1.2; otherwise yeah 3.0.16 would be the preference. Btw, does the branching mean 3.0.x is going to be the maintenance version? I'd like to see whether it's one-time branching only for 3.0.16, or long-term support. If that's latter and Spark community decides to pick 3.0.16, maybe I would try out some fixes like SOE, porting back some bugfixes from 3.1.x into 3.0.x, etc. |
3.0.x is not intended to be long-term supported - it's only a temporary solution until you (or SPARK, or whoever) manages to switch to 3.1.x. |
OK, thanks for explaining. Just to be sure, I'm running Spark UT based on the custom Janino build (mostly identical with master & 3.0.x branches but a bit modified to enable Jitpack). I'll update the result sooner than later. |
FYI, I've succeeded to run Spark UT for both master branch / 3.0.x branch.
|
That's great news! 3.1.x is definitely the way to go, because it will soon produce .class files > Java 6, which is the only way to implement newer language features, esp. default method invocation. |
For Spark project, Janino has been used for compiling "performance-focused generated" code which would be opposite to "hand-written" code so newer language features may not be the project's interest, but I guess the project would prefer going forward with LTS version line once the project decides that the version line looks to be stable. |
Version 3.1.12 is out the door. |
Version 3.0.16 is out the door, too. |
Great! Thanks @aunkrig for handling this so fast! I'll try to integrate these versions once they're available in Nexus and see which version they would prefer. |
Now I've updated my PR in Spark to reflect the official release. Thanks again! |
I'm working on integrating Janino 3.1.1 into Spark as I need to fix #113 in Spark side, but Janino 3.1.1 seems to bring another bunch of test failures which prevents me to persuade upgrading Janino.
Below link is the PR I proposed to upgrade Janino to 3.1.1 in Apache Spark:
apache/spark#27860
and below link is the Jenkins build page which bunch of tests fail due to Janino issue:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/119596/testReport/
I'm now running the Spark tests locally with custom Janino version (3.0.15 + #114 installed locally via
mvn install
) and it seems OK.(There's some conflict for applying #114 to 3.0.15 - I fixed the merge conflict with this commit:
HeartSaVioR@c853b20)
While spending times to investigating and addressing issues on 3.1.x, I'd like to kindly ask about maintaining 3.0.x bugfix versions in parallel. I totally understand the pain on maintaining version lines, so just doing "one time" release for 3.0.x (only 3.0.16) would be also really appreciated.
Thanks in advance!
ps. I succeeded to reproduce one of test failure into Janino UT (will share the details in other issue), though I'm not sure how many test failures are falling into the case and how many cases are remaining.
The text was updated successfully, but these errors were encountered: