apache
Refactor commit logic out of StreamingDataflowWorker
StreamingDataflowWorker is a rather large class
For maintainability it would be helpful in the long run to encapsulate different pieces of the worker harness logic into their own class.
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, commentfixes #<ISSUE NUMBER>instead. - [ ] Update
CHANGES.mdwith 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.
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
R: @scwhittle still working on the tests but curious about initial thoughts. also opted to offload the thread management to executors instead of using raw threads. This may allow us to play around with the thread pool implementation to possibly increase performance (i.e work stealing thread pool).
Thanks!
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
fyi today is release cut. What is the status of this PR? I see the comments are all replied? CC: @scwhittle @m-trieu
There are still some open comments. This is part of work to support direct path which won't be ready for the cut. So I don't think this should hold up the cut.
presubmit failure is due to flaky test (working on a PR to fix) and not related.
added drain (fail work and clear queues) when we close the SEWorkCommitter
will need this for direct path. in non direct path mode we would never call stop (except when we call stop on the entire harness in StreamingDataflowWorker) so it will not affect that code path. @scwhittle
test failures should be fixed in https://github.com/apache/beam/pull/30572 since it removes arbitrary waiting from the tests (replaced with countdownlatch as event triggers)