-
Notifications
You must be signed in to change notification settings - Fork 331
SAMZA-2757: Make Samza Compatible with Java 11 #1628
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
SAMZA-2757: Make Samza Compatible with Java 11 #1628
Conversation
|
@xinyuiscool Sorry to tag you, but I see you were the last one to merge something to this project. Would you be able to take a look at this PR, and the corresponding hello samza one? apache/samza-hello-samza#87 |
…za-to-build-with-java11-real
…james-deee/samza into jtd/get-samza-to-build-with-java11-real
Make changes for some failing issues in tests with Java 11
Issues: Upgrade dependencies for security fixes and enhancements for jetty and jackson In current dependencies there are security vulnerabilities Jackson: https://security.snyk.io/package/maven/com.fasterxml.jackson.core:jackson-databind/2.12.2 Jetty: https://security.snyk.io/package/maven/org.eclipse.jetty:jetty-server/9.4.38.v20210224 Upgrade Jackson to version: 2.13.3 Upgrade Jetty to 9.4.48.v20220622
…pache#1627) Issues: Currently, StreamAppender is not extensible in terms of setting systemProducer and exposing sendEventToSystemProducer to child classes. It also makes it hard for child classes out of package to unit test any logic related to systemProducer Changes: Make StreamAppender extensible 1. Change method sendEventToSystemProducer from private to protected 2. Change member variable systemProducer from private to protected
0d9bc04 to
8058a51
Compare
samza-yarn3/src/test/scala/org/apache/samza/job/yarn/TestSamzaYarnAppMasterLifecycle.scala
Show resolved
Hide resolved
nickpan47
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly lgtm. Please address the few minor comments. Thanks a lot!
samza-log4j2/src/main/java/org/apache/samza/logging/log4j2/StreamAppender.java
Show resolved
Hide resolved
samza-log4j2/src/main/java/org/apache/samza/logging/log4j2/StreamAppender.java
Show resolved
Hide resolved
samza-yarn/src/test/java/org/apache/samza/job/yarn/TestYarnFaultDomainManager.java
Outdated
Show resolved
Hide resolved
samza-yarn/src/test/java/org/apache/samza/job/yarn/TestYarnFaultDomainManager.java
Outdated
Show resolved
Hide resolved
samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaYarnAppMasterLifecycle.scala
Outdated
Show resolved
Hide resolved
…za-to-build-with-java11-real
…the -XX:+PrintGCDateStamps flag
…a/job/yarn/TestYarnFaultDomainManager.java
nickpan47
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The corresponding samza-shell-yarn3 module does not yield a tgz under samza-shell-yarn3/build/distribution directory. Please see my review comments.
build.gradle
Outdated
| task shellTarGz(type: Tar) { | ||
| compression = Compression.GZIP | ||
| classifier = 'dist' | ||
| from 'src/main/bash' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that this works? I pulled down the PR and built locally. Only found that the corresponding samza-shell-yarn3 build does not yield any tgz file as output in build/distribution, since there is no separate samza-shell-yarn3/src in the source repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the following change here and the build command successfully generates the samza-shell-yarn3 .tgz file:
`
- from 'src/main/bash'
- from 'src/main/resources'
- from 'src/main/visualizer'
- from(project(":samza-shell").file("./src/main/bash"))
- from(project(":samza-shell").file("./src/main/resources"))
- from(project(":samza-shell").file("./src/main/visualizer"))
`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh jeez. what a dummy i am. Should probably make sure the actual distribution get's build 🤦 .
Thanks! I updated it.
nickpan47
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for your contribution!
Feature: Make Samza build artifacts runnable in JDK11 Changes: - introduced new build modules samza-yarn3 and samza-shell-yarn3 to build Samza w/ YARN 3.3.4, which is runtime compatible w/ JDK11 Tests: - local test and hello-samza succeeded with Java11
This reverts commit 0396739.
Feature: Make Samza build artifacts runnable in JDK11 Changes: - introduced new build modules samza-yarn3 and samza-shell-yarn3 to build Samza w/ YARN 3.3.4, which is runtime compatible w/ JDK11 Tests: - local test and hello-samza succeeded with Java11
This reverts commit 5580bcf. RB=3722433 G=stream-processing-reviewers A=xiliu,alazhang
Issues: The current Samza Project does not support running in a Java 11 runtime environment. It will continue to build this project with Java 8, however.
Changes:
samza-yarn3andsamza-shell-yarn3project to support people wanting to run in Java 11 environments with the required Yarn version of 3.3.+ (which is when Yarn started supporting Java 11).org.apache.commons.lang3Tests:
samza-yarn3module.API Changes: None
Upgrade Instructions: All current users of Samza should be able to continue using Samza as usual, there is nothing required for upgrading to this new version.
Usage Instructions: If you want to use Java 11, then you should make use of the
samza-yarn3andsamza-shell-yarn3modules. The following modules do NOT support Yarn 3:samza-testsamza-hdfssamza-yarn