Re: [NOTICE] Experimental Java 17 support now available on master
Hi Matthias, Thanks for the information. Best regards, Jing On Thu, Sep 7, 2023 at 5:36 PM Matthias Pohl 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 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 >> 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 >>> 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
Re: [NOTICE] Experimental Java 17 support now available on master
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 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 > 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 >> 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
Re: [NOTICE] Experimental Java 17 support now available on master
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 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 > 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 > > >
Re: [NOTICE] Experimental Java 17 support now available on master
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 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.
Re: [NOTICE] Experimental Java 17 support now available on master
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 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.
