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

Enable external callers to start a WindmillStream #32763

Closed
wants to merge 27 commits into from

Conversation

m-trieu
Copy link
Contributor

@m-trieu m-trieu commented Oct 12, 2024

WindmillStream implementations were previously created and returned in a "started" state usually after the stream has already sent the initial headers to open the connection to the backend servers.

This prevents us from being able to start the stream "lazily".

Add flexibility to the WindmillStream API by allowing external callers to start the stream themselves. This is especially important in direct path mode where the user worker manages the fan out to the backend.

in terms of implementation, similar to WindmillStream.shutdown(), WindmillStream.start()'s behavior will only execute once during the lifetime of the WindmillStream object. Subsequent calls to start() will do nothing.

branched off of #31902, should be merged after

changes to relevant to this PR are in commit ec9dd32

R: @scwhittle @arunpandianp


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.

m-trieu added 26 commits October 9, 2024 18:32
… have it implement WorkProvider interface. Move class to windmill/work/provider directory, update visibility for dependent classes and move tests, add GetWorkBudgetOwnerInterface
… have it implement WorkProvider interface. Move class to windmill/work/provider directory, update visibility for dependent classes and move tests, add GetWorkBudgetOwnerInterface
… have it implement WorkProvider interface. Move class to windmill/work/provider directory, update visibility for dependent classes and move tests, add GetWorkBudgetOwnerInterface
… WindmillEndpoints and don't process any version that is older than the current version in FanOutStreamingEngineWorkerHarness
…ady 600 seconds anyway. Move DEFAULT_STREAM_RPC_DEADLINE_SECONDS to where it is being used and remove references in tests
…e StreamingEngineConnectionsState to StreamingEngineBackends
Copy link
Contributor

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

… around in an unstarted state. Start the streams in WindmillStreamSender in parallel
@m-trieu m-trieu closed this Nov 21, 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.

1 participant