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.

Reply via email to