Hi Matthias, Thanks for the information.
Best regards, Jing On Thu, Sep 7, 2023 at 5:36 PM Matthias Pohl <matthias.p...@aiven.io> wrote: > I don't know about the specifics of the kryo 2.x vs 5.x issues. The > discussion around FLIP-317 [1] seems to have stalled here. But Java 17 is > still considered an experimental feature as expressed in Flink's roadmap > [2]. So, it should be fine to have it in 1.18. I updated the release notes > of FLINK-15736 [3] to reflect the beta stage of this feature. Does that > sound reasonable? > > Matthias > > [1] > https://cwiki.apache.org/confluence/display/FLINK/FLIP-317%3A+Upgrade+Kryo+from+2.24.0+to+5.5.0 > [2] https://flink.apache.org/roadmap/ > [3] https://issues.apache.org/jira/browse/FLINK-15736 > > On Wed, Sep 6, 2023 at 11:26 PM Jing Ge <j...@ververica.com> wrote: > >> 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. >>> >>
