Added support for SparkRunner streaming stateful processing #33267
Conversation
- and define it as AutoValue class
…to TranslationUtils - add rejectTimers method
- refactor : remove @nullable annotation in static factory constructor
- change SparkMapState logic to provide NPE - change addTimers to public modifier
…or performance) - change ParDoStateUpdateFn to use SparkTimerInternals - reorder ParDoStateUpdateFn state.update
- update CHANGES.md
twosom
changed the title
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
Run Java PreCommit |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @lostluck added as fallback since no labels match configuration Available commands:
The PR bot will only process comments in the main thread (not review comments). |
twosom
changed the title
|
R: @kennknowles |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
Ah can you add a small change to these files, which will cause the ValidatesRunner tests for stateful processing to run?
I don't know the codebase enough at this point to know what is the sharing between the things tested by these jobs. (incidentally there is a |
|
i touched trigger files! |
|
Run Java_Spark3_Versions PreCommit |
|
Hi! Thanks a lot! |
|
Acknowledged! I'm on it. |
There was a problem hiding this comment.
Thanks for this contribution! I admit that I am leaning heaviliy on the testing since I'm a bit rusty on the runner internals and the Spark details, and I don't have time right now to page it all in. I don't want to slow down your contribution any further. I think we can merge, and I just left a small comment which you might adjust in a follow-up PR.
|
|
||
| rejectTimers(doFn); | ||
| checkArgument( | ||
| !signature.processElement().isSplittable(), |
There was a problem hiding this comment.
Splittable and stateful are also just not compatible, so this situation should be impossible.
There was a problem hiding this comment.
You're right! I'll remove this redundant check in a follow-up PR since splittable and stateful are mutually exclusive by design. Thank you for catching this. 😀
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #33267 +/- ##
=========================================
Coverage 57.37% 57.37%
Complexity 1475 1475
=========================================
Files 973 973
Lines 154905 154918 +13
Branches 1076 1076
=========================================
+ Hits 88879 88887 +8
- Misses 63810 63821 +11
+ Partials 2216 2210 -6 ☔ View full report in Codecov by Sentry. |
Please add a meaningful description for your change here
fixes #33237
This PR contains these changes
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
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.CHANGES.mdwith noteworthy changes.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.