[acrites]

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.

[github-actions]

📋 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, and StreamingModeExecutionContextTest.
• Calling getAndReset() on the shared Counter when multiple SDF ParDoOperations exist in the same MapTask works safely (since the first operation drains the shared counter and subsequent ones return 0), and the added comment in StreamingModeExecutionContext helpful documents the existing interaction with StreamingWorkerStatusReporter.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[codecov]

Codecov Report

❌ Patch coverage is 90.69767% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.17%. Comparing base (76f6e6d) to head (9f6dfe8).
⚠️ Report is 280 commits behind head on master.

Files with missing lines	Patch %	Lines
...dataflow/worker/StreamingModeExecutionContext.java	90.00%	0 Missing and 2 partials ⚠️
...taflow/worker/IntrinsicMapTaskExecutorFactory.java	88.88%	0 Missing and 1 partial ⚠️
...low/worker/util/common/worker/MapTaskExecutor.java	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #40639       +/-   ##
=============================================
+ Coverage     56.16%   76.17%   +20.01%     
+ Complexity     2288     2286        -2     
=============================================
  Files          1124      259      -865     
  Lines        178336    14408   -163928     
  Branches       1489     1488        -1     
=============================================
- Hits         100170    10976    -89194     
+ Misses        75645     2737    -72908     
+ Partials       2521      695     -1826     

Flag	Coverage Δ
go	?
java	76.17% <90.69%> (+0.39%)	⬆️
python	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Assigning reviewers:

R: @kennknowles added as fallback since no labels match configuration
Backup reviewer: damccorm (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

[scwhittle]

why track just the first receiver? If a operator is multioutput are we missing tracking some of it?

[acrites]

I'm not entirely sure. I didn't actually change this code, just added some extra null checks. I guess as long as the SplittableFn is not outputting the same element across multiple outputs we don't risk double-counting. I think that might more closely match what the portable worker does.

[scwhittle]

do we want to sample this often? should it be time-based?

[acrites]

I'm happy to change it if we can agree on a better time. This is just what we currently do for custom source reporting already.

[scwhittle]

this seems brittle. I'm not very familiar with this code but perhaps Kenn will have a cleaner way of doing this.

[acrites]

My first thought was to add something more generically to ParDo, but I feel that this is more of a Dataflow implementation detail that we even need to do this, so I'd rather not add something to the generic API to specify which ParDo's should be reporting their bytes to autoscaling. Maybe there's some way to add something to the CloudObject in SplittableProcessFnFactory.java?

[acrites]

Here were some alternatives provided by AI, though it's first recommendation is what I didn't really want to do:

1. Add a capability method to ParDoFn (Recommended)

Add default boolean isSplittable() { return false; } (or shouldReportBytesRead()) on ParDoFn (and delegate in ForwardingParDoFn).
Determine it via the actual DoFn class rather than any string: SplittableProcessFnFactory.ProcessFnExtractor wraps the user DoFn in SplittableParDoViaKeyedWorkItems.ProcessFn, which is already accessible at construction time via helpers.doFnInstanceManager.peek().getDoFn() instanceof ProcessFn inside SimpleParDoFnHelpers (used by both SimpleParDoFn and StreamingKeyedWorkItemSideInputParDoFn).
Alternatively, SplittableProcessFnFactory.createDefault() (or UserParDoFnFactory.streamingKeyedWorkItem, which is already true exclusively for SplittableProcessFnFactory) can set/wrap that flag on the returned ParDoFn.
IntrinsicMapTaskExecutorFactory then simply checks if (fn.isSplittable()), keeping all CloudObject dispatch encapsulated inside DefaultParDoFnFactory.

2. Have SplittableProcessFnFactory wrap with a dedicated ForwardingParDoFn subclass (or marker interface)

If you don't want to touch the public ParDoFn interface, SplittableProcessFnFactory can wrap the ParDoFn produced by UserParDoFnFactory in a package-private SplittableParDoFn extends ForwardingParDoFn (or marker interface), allowing fn instanceof SplittableParDoFn in IntrinsicMapTaskExecutorFactory.

3. Share a constant if keeping the CloudObject check

If you prefer keeping the check on cloudUserFn.getClassName(), extracting a shared constant (e.g., SplittableProcessFnFactory.OBJECT_TYPE_NAME = "SplittableProcessFn") used by DefaultParDoFnFactory, IntrinsicMapTaskExecutorFactory, and tests avoids duplicating the string literal (similar to ComputationWorkExecutorFactory.isCustomSource), though Option 1 or 2 is cleaner since it delegates the classification to ParDoFnFactory.

[kennknowles]

R: @scwhittle

[github-actions]

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers