Repository navigation
[Java Harness] Handle swallowed exceptions across fused DoFns - #41285
knockknock10 wants to merge 37 commits into
Conversation
There was a problem hiding this comment.
📋 Review Summary
This PR adds the fused_dofn_exception_handling experiment (legacy, warn, fail) to detect and handle exceptions thrown by downstream consumers that are caught and swallowed by upstream fused DoFns. While the core tracking in PCollectionConsumerRegistry and ProcessBundleHandler is well-structured, there are a few correctness issues regarding bundle failure timing (running @FinishBundle after a downstream failure), sibling consumer blocking in multiplexing receivers, and unchecked Error handling.
🔍 General Feedback
- Bundle lifecycle on failure: In
ProcessBundleHandler,failBundleIfDownstreamFailed()is currently invoked afterfinishFunctionRegistryruns. Infailmode, if a downstream transform failed during element processing, running@FinishBundleon the broken transform (which can flush invalid partial state or throw a secondary masking exception) should be avoided. - Multiplexing consumer isolation: In
MultiplexingMetricTrackingFnDataReceiver, callingfailFastIfDownstreamFailedinside the consumer loop aborts the entire loop on subsequent elements, preventing healthy sibling consumers of the samePCollectionfrom receiving elements before the exception is rethrown to the upstreamDoFn. - Catching
Throwable:User code can throwErrorsubclasses (e.g.,AssertionError) and catchThrowableorErrorupstream; trackingExceptiononly misses these cases.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @chamikaramj for label java. This pull request likely touches a core component ("core" label). Please review with scrutiny. 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). |
This PR addresses #34419.
What problem does this solve?
When two DoFns are fused, an exception thrown by a downstream DoFn can propagate through the upstream DoFn's
output()call. If the upstream DoFn catches that exception, the bundle may otherwise complete successfully even though downstream processing failed.This change introduces configurable handling for exceptions thrown across fused DoFn boundaries in the Java Fn harness.
Implementation
Adds the
fused_dofn_exception_handlingexperiment with three modes:legacy: Preserves the existing exception propagation behavior.warn: Preserves exception propagation, but logs a warning if a downstream exception was swallowed and the bundle would otherwise complete successfully.fail: Records downstream exceptions, prevents subsequent outputs to the affected downstream transform, and fails the bundle if it would otherwise complete successfully.Exceptions are recorded at the consumer boundary. The selected policy is applied during bundle processing and completion, and the recorded exception state is reset when the bundle processor is reset.
Tests
Adds registry-level and bundle-level regression tests covering:
failmode.Errorsubclasses.@FinishBundleexecution after a swallowed processing failure.Validation
Executed locally using Temurin JDK 21:
Results:
ProcessBundleHandlerTestandPCollectionConsumerRegistryTest:The branch was rebased onto Apache Beam
masterbefore validation.