Repository navigation
Conversation
There was a problem hiding this comment.
📋 Review Summary
This PR enables reporting of bytes processed for Splittable DoFns (SplittableProcessFn) in the Dataflow streaming worker so that autoscaling can accurately estimate backlog clearance time alongside SDF backlog reporting. The implementation cleanly propagates the output coder to ParDoOperation, attaches the byte counter in ComputationWorkExecutorFactory, and aggregates the processed bytes in StreamingModeExecutionContext with thorough unit test coverage.
🔍 General Feedback
- The changes are well-structured, adhere to existing worker conventions, and include comprehensive unit tests across
IntrinsicMapTaskExecutorFactoryTest,MapTaskExecutorTest,ComputationWorkExecutorFactoryTest, andStreamingModeExecutionContextTest. - Calling
getAndReset()on the sharedCounterwhen multiple SDFParDoOperations exist in the sameMapTaskworks safely (since the first operation drains the shared counter and subsequent ones return0), and the added comment inStreamingModeExecutionContexthelpful documents the existing interaction withStreamingWorkerStatusReporter.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #40639 +/- ##
=============================================
+ Coverage 56.16% 76.03% +19.86%
- Complexity 2288 2301 +13
=============================================
Files 1124 259 -865
Lines 178336 14490 -163846
Branches 1489 1496 +7
=============================================
- Hits 100170 11018 -89152
+ Misses 75645 2766 -72879
+ Partials 2521 706 -1815
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @kennknowles added as fallback since no labels match configuration Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
SDF's SplittableProcessFn's report their backlog for autoscaling, so we need to also report their bytes processed for autoscaling to correctly compute time to clear backlog.
SplittableProcessFn's are tagged when constructing MapTaskExecutors and their output coders are attached to the ParDoOperation so the counter values can be updated while processing.
Also adds tests for the new code.