Skip to content

Flink: Backport add StreamingStartingStrategy.INCREMENTAL_FROM_LATEST_SNAPSHOT_EXCLUSIVE to Flink 1.19 #12899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 28, 2025

Conversation

morhidi
Copy link
Contributor

@morhidi morhidi commented Apr 25, 2025

backports #12839

@github-actions github-actions bot added the flink label Apr 25, 2025
@morhidi morhidi force-pushed the latest-exclusive-1.19 branch from 22e543f to e400e38 Compare April 25, 2025 22:37
@morhidi
Copy link
Contributor Author

morhidi commented Apr 25, 2025

cc @rodmeneses @mxm

@morhidi morhidi force-pushed the latest-exclusive-1.19 branch from e400e38 to f3e8292 Compare April 26, 2025 01:31
@morhidi morhidi changed the title Flink: Backport add StreamingStartingStrategy.INCREMENTAL_FROM_LATEST_SNAPSHOT_EXCLUSIVE Flink: Backport add StreamingStartingStrategy.INCREMENTAL_FROM_LATEST_SNAPSHOT_EXCLUSIVE to Flink 1.19 Apr 26, 2025
Copy link
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Guosmilesmile
Copy link
Contributor

Hi, @mxm I have a small question regarding future backports. Since Flink 2.0 has been introduced, should we also prepare new backports for the 2.0 version going forward?

@mxm
Copy link
Contributor

mxm commented Apr 26, 2025

@Guosmilesmile Absolutely, there was a bit of race condition with merging the original PR and the Flink 2.0 PR. Thanks for pointing that out!

@morhidi Could you also include the Flink 2.0 changes in this PR? Normally, we merge to the latest Flink version first and then backport, but this is a special situation.

@Guosmilesmile
Copy link
Contributor

@mxm Thank you very much for your quick response. Since 2.0 was just merged a few days ago, I also had the same confusion. Now I understand how to proceed in the future. Thank you very much!

@morhidi
Copy link
Contributor Author

morhidi commented Apr 26, 2025

@Guosmilesmile Absolutely, there was a bit of race condition with merging the original PR and the Flink 2.0 PR. Thanks for pointing that out!

@morhidi Could you also include the Flink 2.0 changes in this PR? Normally, we merge to the latest Flink version first and then backport, but this is a special situation.

Flink 2.0 contains these changes already, only Flink 1.19 does not, so I we're good here

@morhidi
Copy link
Contributor Author

morhidi commented Apr 27, 2025

@stevenzwu or @pvary could you please check and merge this backport?

@pvary
Copy link
Contributor

pvary commented Apr 28, 2025

@morhidi: is this a clean backport?

Please help future reviewers by starting if a backport is clean, or highlight the required changes.

Thanks, Peter

@mxm
Copy link
Contributor

mxm commented Apr 28, 2025

@morhidi @Guosmilesmile Indeed, 2.0 already contains this change. I rebased the Flink 2.0 PR (#12527) based on the Flink 1.20 code after the original PR to this backport (#12839) had been merged. So we're good for Flink 2.0.

@Guosmilesmile
Copy link
Contributor

@mxm @morhidi Thank you very much for your answers.

@morhidi
Copy link
Contributor Author

morhidi commented Apr 28, 2025

@morhidi: is this a clean backport?

Please help future reviewers by starting if a backport is clean, or highlight the required changes.

Thanks, Peter

This is 'clean' @pvary a new startup mode was introduced + covered with extra unit tests.

@pvary pvary merged commit eed6dc7 into apache:main Apr 28, 2025
20 checks passed
@pvary
Copy link
Contributor

pvary commented Apr 28, 2025

Discussed with @morhidi offline. Clean means, that there are no changes compared to the original PR. This backport is clean.
Merged to main.
Thanks @morhidi for the PR and @Guosmilesmile and @mxm for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants