Hi folks, Sorry to come back to this thread, since I saw FLINK-32327 is already closed and Chesnay announced on 16th June that Flink master branch (at that time, now it should be both 1.18 and master branches) builds and runs with Java 17 out-of-the-box. But according to the feedback from Kurt, I'd like to double confirm with the following question:
@Chesnay @Mathias: Will the upcoming Flink 1.18.0 release support Java 17 even if there are known issues with Java records because of kryo 2.x? Thanks! Best regards, Jing On Sun, Jun 18, 2023 at 8:45 PM Kurt Ostfeld <kurtostf...@proton.me> wrote: > I think there is some confusion: > > Chesnay, not me, recently checked in changes into master so that Flink > will build + test + run with experimental support for Java 17 but with Kryo > 2.x as-is so this will error with Java records. Chesnay created this > particular email thread related to this work. > > I (Kurt), created a PR+FLIP several weeks ago for upgrading Kryo from 2.x > to 5.x, with full backward compatibility for existing > savepoints/checkpoints, that enables Flink to run on Java 17 with support > for Java records. This isn't merged into master. I haven't gotten much > feedback on this. > > I recently rebased the Kryo upgrade PR onto the master branch, whicch > includes Chesnay commits. The PR branch was already running successfully on > Java 17, Chesnay's changes enable Flink to build and run the CI test suite > in Java 17 as well. However, without the Kryo upgrade, Flink isn't > compatible with Java records. > > I'd be happy to follow the standard process and do the the FLIP vote, but > before this is ready for a vote, this PR needs review + testing by someone > other than me. Specifically, I'd like someone to try to create a Flink > application that tries to break the upgrade process: either confirm that > everything works or demonstrate an error scenario. > > The Kryo PR code is passing all automated CI tests, which include several > tests covering backwards compatibility scenarios. I also created this > simple application https://github.com/kurtostfeld/flink-kryo-upgrade-demo > to create state with Flink 1.17 and test the upgrade process. From what I > can see it works, but this would definitely need more testing from people > other than just me. > > > > ------- Original Message ------- > On Sunday, June 18th, 2023 at 7:41 AM, Jing Ge <j...@ververica.com.INVALID> > wrote: > > > > > > > > Hi Kurt, > > > > Thanks for your contribution. I am a little bit confused about the email > > title, since your PR[1] is not merged into the master yet. I guess, with > > "Experimental Java 17 support", you meant it is available on your branch > > which is based on the master. > > > > If I am not mistaken, there is no vote thread of FLIP 317 on ML. Would > you > > like to follow the standard process[2] defined by the Flink community? > > Thanks! > > > > > > Best regards, > > Jing > > > > [1] https://github.com/apache/flink/pull/22660 > > [2] > > > https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals > > > > On Sun, Jun 18, 2023 at 1:18 AM Kurt Ostfeld > kurtostf...@proton.me.invalid > > > > wrote: > > > > > I built the Flink master branch and tried running this simple Flink app > > > that uses a Java record: > > > > > > > https://github.com/kurtostfeld/flink-kryo-upgrade-demo/blob/main/flink-record-demo/src/main/java/demo/app/Main.java > > > > > > It fails with the normal exception that Kryo 2.x throws when you try to > > > serialize a Java record. The full stack trace is here: > > > https://pastebin.com/HGhGKUWt > > > > > > I tried removing this line: > > > > > > > https://github.com/kurtostfeld/flink-kryo-upgrade-demo/blob/main/flink-record-demo/src/main/java/demo/app/Main.java#L36 > > > > > > and that had no impact, I got the same error. > > > > > > In the other thread, you said that the plan was to use PojoSerializer > to > > > serialize records rather than Kryo. Currently, the Flink code bases > uses > > > Kryo 2.x by default for generic user data types, and that will fail > when > > > the data type is a record or contains records. Ultimately, if Flink > wants > > > to fully support Java records, it seems that it has to move off of Kryo > > > 2.x. PojoSerializer is part of what is basically a custom serialization > > > library internal to Flink that is an alternative to Kryo. That's one > > > option: move off of Kryo to a Flink-internal serialization library. The > > > other two options are upgrade to the new Kryo or use a different > > > serialization library. > > > > > > The Kryo 5.5.0 upgrade PR I submitted ( > > > https://github.com/apache/flink/pull/22660) with FLIP 317 ( > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-317%3A+Upgrade+Kryo+from+2.24.0+to+5.5.0 > ) > > > works with records. The Flink app linked above that uses records works > with > > > the PR and that's what I posted to this mailing list a few weeks ago. I > > > rebased the pull request on to the latest master branch and it's > passing > > > all tests. From my testing, it supports stateful upgrades, including > > > checkpoints. If you can demonstrate a scenario where stateful upgrades > > > error I can try to resolve that. >
