Add support for Flink 1.19 #32648

kennknowles · 2024-10-03T19:41:41Z

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

github-actions · 2024-10-16T18:39:32Z

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

kennknowles · 2024-10-16T21:08:59Z

Failing test is broken at HEAD and doesn't touch Flink.

je-ik

LGTM, cool!

I found some outdated references to flink beam_flink1.10 in learning/tour-of-beam/learning-content/introduction/introduction-concepts/runner-concepts/description.md, we might want to update those as well?

We are apparently using Flink 1.15 for portable tests (image beam_flink1.15).

There is also an issue #31631, do we want to resolve this as part of 2.61.0 release (mark this issue as release blocker for 2.61.0)? That way, this could be the last time we need to update the repo.

Last note, we might add an issue to drop Flink 1.15 and 1.16, the current policy is to support three versions of Flink.

je-ik · 2024-10-17T06:39:19Z

.github/trigger_files/beam_PostCommit_Java_ValidatesRunner_Flink_Java11.json

@@ -1,4 +1,5 @@
 {
  "comment": "Modify this file in a trivial way to cause this test suite to run",
-  "https://github.com/apache/beam/pull/31156": "noting that PR #31156 should run this test"
+  "https://github.com/apache/beam/pull/31156": "noting that PR #31156 should run this test",
+  "https://github.com/apache/beam/pull/32648": "testing addition of Flink 1.19 support"


Unrelated, but these edits seem to be accumulating over time. Can we (probably in a different issue and PR) add a script that will just update some timestamp field of the json?

Yea, we could. But TBH it kind of doesn't matter? The original idea is we would drop the commit before merging. But of course most people are not very careful about their commit history. Some people like to put a meaningless entry. I like to put an entry that describes the test so I don't mind if it gets checked in. We can also just trigger the tests by deleting all the entries, too.

je-ik · 2024-10-17T06:46:27Z

runners/flink/1.19/src/test/java/org/apache/beam/runners/flink/streaming/StreamSources.java

+            operator.getOperatorConfig(), new MockEnvironmentBuilder().build()));
+  }
+
+  /** The emitWatermarkStatus method was added in Flink 1.14, so we need to wrap Output. */


Flink 1.14 is no longer supported, seems that we currently need the wrapper because of difference between Flink 1.15-1.18 and 1.19, am I correct?

Yes, that's right. Since we dropped 1.14 support we could eliminate this default and inline it to all the uses of the OutputWrapper. I will probably do some of this clean up since I am learning this area of the code right now.

je-ik · 2024-10-17T06:49:58Z

runners/flink/1.19/src/test/java/org/apache/beam/runners/flink/streaming/StreamSources.java

+    /** In Flink 1.19 the {@code emitRecordAttributes} method was added. */
+    @Override
+    default void emitRecordAttributes(RecordAttributes recordAttributes) {
+      throw new UnsupportedOperationException("emitRecordAttributes not implemented");


Given the implementation, we could move the StreamSources to src/main (shared across versions), if we just drop the @Override annotation.

Agree, we could. I'm still paging in how the FlinkRunner code is organized so right now I am just copying what already exists. Actually I started by making a new adapter before I saw that this already exists, so I am just keeping it where it already was for now.

kennknowles

Thanks!

Yes, let me make some notes and I will follow up on these pieces as well:

update flink1.10 and flink1.15 references
drop 1.15 (not sure about dropping two versions in a single release - would like to get an idea of usage)
take a look and see if the dockerhub thing is low-hanging fruit or more difficult (maybe you already know?)

kennknowles · 2024-10-17T11:30:46Z

.github/trigger_files/beam_PostCommit_Java_ValidatesRunner_Flink_Java11.json

@@ -1,4 +1,5 @@
 {
  "comment": "Modify this file in a trivial way to cause this test suite to run",
-  "https://github.com/apache/beam/pull/31156": "noting that PR #31156 should run this test"
+  "https://github.com/apache/beam/pull/31156": "noting that PR #31156 should run this test",
+  "https://github.com/apache/beam/pull/32648": "testing addition of Flink 1.19 support"


Yea, we could. But TBH it kind of doesn't matter? The original idea is we would drop the commit before merging. But of course most people are not very careful about their commit history. Some people like to put a meaningless entry. I like to put an entry that describes the test so I don't mind if it gets checked in. We can also just trigger the tests by deleting all the entries, too.

kennknowles · 2024-10-17T11:33:13Z

runners/flink/1.19/src/test/java/org/apache/beam/runners/flink/streaming/StreamSources.java

+            operator.getOperatorConfig(), new MockEnvironmentBuilder().build()));
+  }
+
+  /** The emitWatermarkStatus method was added in Flink 1.14, so we need to wrap Output. */


Yes, that's right. Since we dropped 1.14 support we could eliminate this default and inline it to all the uses of the OutputWrapper. I will probably do some of this clean up since I am learning this area of the code right now.

kennknowles · 2024-10-17T11:34:28Z

runners/flink/1.19/src/test/java/org/apache/beam/runners/flink/streaming/StreamSources.java

+    /** In Flink 1.19 the {@code emitRecordAttributes} method was added. */
+    @Override
+    default void emitRecordAttributes(RecordAttributes recordAttributes) {
+      throw new UnsupportedOperationException("emitRecordAttributes not implemented");


Agree, we could. I'm still paging in how the FlinkRunner code is organized so right now I am just copying what already exists. Actually I started by making a new adapter before I saw that this already exists, so I am just keeping it where it already was for now.

kennknowles · 2024-10-17T11:39:12Z

My intention is to follow this up with Flink 1.20 support as well, since I want to get us caught up with current. So then the whole "drop 1.15, 1.16, 1.17 support" issue needs some proper handling and not too rushed.

je-ik · 2024-10-17T12:15:26Z

take a look and see if the dockerhub thing is low-hanging fruit or more difficult (maybe you already know?)

Unfortunately no. I'm not too much familiar with how we build the job server images, but I suppose, it should be only changing the image name and bundling Flink version into the tag. There are reference to "latest' though, which would be meaningless. We would likely have to stop pushing those and create tags like '1
19_dev', which would be pushed for every version as we do with 'latest' now. But that's all I have.

kennknowles · 2024-10-17T12:46:58Z

take a look and see if the dockerhub thing is low-hanging fruit or more difficult (maybe you already know?)

Unfortunately no. I'm not too much familiar with how we build the job server images, but I suppose, it should be only changing the image name and bundling Flink version into the tag. There are reference to "latest' though, which would be meaningless. We would likely have to stop pushing those and create tags like '1 19_dev', which would be pushed for every version as we do with 'latest' now. But that's all I have.

OK. I will think about this as a background task.

On the subject of the Flink versions, I do see that in the past we added one version and removed two all in the same Beam release. Since the Flink site themselves only even has the downloads for 1.18 and later, I think we can move quickly eliminating versions that are not longer supported by Flink. So I will do 1.15 and 1.16 now, and also add 1.20. I don't think I'll remove 1.17 until after 2.61.0 is released.

kennknowles · 2024-10-17T12:55:02Z

Noting https://issues.apache.org/jira/browse/INFRA-26211 to add the new dockerhub repo.

And, indeed, noting the specific files that were not updated here and are behind:

.github/workflows/beam_LoadTests_Java_GBK_Smoke.yml
.github/workflows/beam_PostCommit_Java_Examples_Flink.yml
.github/workflows/beam_PostCommit_Java_Nexmark_Flink.yml
.github/workflows/beam_PostCommit_Java_PVR_Flink_Streaming.yml
.github/workflows/beam_PostCommit_Java_ValidatesRunner_Flink_Java8.yml
.github/workflows/beam_PostCommit_XVR_Flink.yml
.github/workflows/beam_PreCommit_Java_PVR_Flink_Batch.yml
.github/workflows/beam_PreCommit_Java_PVR_Flink_Docker.yml
learning/tour-of-beam/learning-content/introduction/introduction-concepts/runner-concepts/description.md

(this comment will be edited to have the full list of files that referenced 1.15)

I am updating these to 1.19 in #32837 and #32836

github-actions bot added go build examples runners flink labels Oct 3, 2024

kennknowles force-pushed the flink_1.19 branch from f2f6e33 to 164fe70 Compare October 3, 2024 19:56

Touch Flink trigger files for testing addition of Flink 1.19 support

19044ec

kennknowles force-pushed the flink_1.19 branch from 164fe70 to 7298f28 Compare October 3, 2024 20:00

github-actions bot added python website typescript labels Oct 3, 2024

kennknowles mentioned this pull request Oct 3, 2024

[flink-runner] Improve Datastream for batch performances #32440

Open

3 tasks

kennknowles force-pushed the flink_1.19 branch 3 times, most recently from a4629f1 to bafa3ff Compare October 16, 2024 00:43

kennknowles added 2 commits October 16, 2024 13:23

Pin snakeyaml-engine to 2.6 so it is not downgraded by transitive deps

aa74cc0

Add support for Flink 1.19

3978922

kennknowles force-pushed the flink_1.19 branch from bafa3ff to 3978922 Compare October 16, 2024 17:23

kennknowles requested a review from je-ik October 16, 2024 17:33

kennknowles marked this pull request as ready for review October 16, 2024 17:33

kennknowles requested a review from Abacn October 16, 2024 18:57

je-ik approved these changes Oct 17, 2024

View reviewed changes

kennknowles commented Oct 17, 2024

View reviewed changes

kennknowles merged commit ba92381 into apache:master Oct 17, 2024
136 of 140 checks passed

kennknowles deleted the flink_1.19 branch October 17, 2024 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Flink 1.19 #32648

Add support for Flink 1.19 #32648

kennknowles commented Oct 3, 2024

github-actions bot commented Oct 16, 2024

kennknowles commented Oct 16, 2024

je-ik left a comment

je-ik Oct 17, 2024

kennknowles Oct 17, 2024

je-ik Oct 17, 2024

kennknowles Oct 17, 2024

je-ik Oct 17, 2024

kennknowles Oct 17, 2024

kennknowles left a comment

kennknowles Oct 17, 2024

kennknowles Oct 17, 2024

kennknowles Oct 17, 2024

kennknowles commented Oct 17, 2024

je-ik commented Oct 17, 2024

kennknowles commented Oct 17, 2024

kennknowles commented Oct 17, 2024 •

edited

Loading

Add support for Flink 1.19 #32648

Add support for Flink 1.19 #32648

Conversation

kennknowles commented Oct 3, 2024

GitHub Actions Tests Status (on master branch)

github-actions bot commented Oct 16, 2024

kennknowles commented Oct 16, 2024

je-ik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kennknowles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kennknowles commented Oct 17, 2024

je-ik commented Oct 17, 2024

kennknowles commented Oct 17, 2024

kennknowles commented Oct 17, 2024 • edited Loading

kennknowles commented Oct 17, 2024 •

edited

Loading