Skip to content
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

[WIP] Flink unified translation #28944

Closed
wants to merge 38 commits into from
Closed

Conversation

jto
Copy link
Contributor

@jto jto commented Oct 11, 2023

This PR provides a new "unified" implementation of a translator for Flink capable of translating all workflows, portable or not, streaming and batch.

It replaced the 4 implementations that currently exist with the following advantages:

  • Uses the new DataStream API rather that the deprecated DataSet.
  • Uses the new Flink source implementation rather that the deprecated InputFormat and SourceFunction
  • Easier to maintain. There's a lot of overlap between the existing implementation and code is largely copy pasted from each other (with subtle accidental variations)

fixes #28617

  • Use Unified translator for native streaming pipelines
  • Use Unified translator for native batch pipelines when --useDataStreamForBatch is passed
  • Use Unified translator for portable streaming pipelines
  • Use Unified translator for portable batch pipelines

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)

Build python source distribution and wheels
Python tests
Java tests
Go tests

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

@jto jto force-pushed the flink_unified_translation branch from c00e69c to 57b8e9e Compare October 11, 2023 12:34
@jto jto force-pushed the flink_unified_translation branch from 5394c83 to 60a3024 Compare October 24, 2023 12:52
@jto jto force-pushed the flink_unified_translation branch 5 times, most recently from 7edf597 to c3dd3e1 Compare October 24, 2023 15:06
@github-actions github-actions bot removed the samza label Oct 24, 2023
@jto jto force-pushed the flink_unified_translation branch from 4b468f6 to 02f7cbc Compare October 25, 2023 09:54
@jto jto force-pushed the flink_unified_translation branch from 2de67b3 to ce1d317 Compare November 23, 2023 10:12
@jto
Copy link
Contributor Author

jto commented Dec 1, 2023

There's currently an issue in Batch mode specifically with timers because of a race condition in ExecutableStageDoFnOperator.

Because processElement is async and the actual waiting for bundle being processed only happens in finishBundle, the following issue 2 issues will happen:

  • Timers need to be forcibly triggered at the end of the bundle in batch mode otherwise they may be lost. This is easily fixed because a similar hack exists for processing time events.

  • A DoFn that sets a timer will execute and its corresponding bundle will actually finish, however, while the DoFn processElement is successfully executed the actual registration of the timer is not yet completed (everything is async since the body of the DoFn runs in a different process / thread). The following events will happen:

    1. Bundle finishes,
    2. Timers are set and fired,
    3. next bundle is started.

    Because the registering and firing of timers happens BETWEEN 2 bundles (after finish, before start) those timers firing will just be lost.

Copy link
Contributor

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jan 31, 2024
Copy link
Contributor

github-actions bot commented Feb 8, 2024

This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Merge Flink runner translations ?
1 participant