Repository navigation
Adding skip shuffle shared guard - #172
Merged
Merged
Conversation
yanxinyi
force-pushed
the
xiyan/skip-shuffle-shared-guard
branch
from
September 15, 2026 21:29
4e9ca27 to
7fb1b68
Compare
yanxinyi
marked this pull request as ready for review
September 15, 2026 21:58
yanxinyi
force-pushed
the
xiyan/skip-shuffle-shared-guard
branch
2 times, most recently
from
September 17, 2026 22:28
b1c5e3b to
d3d5369
Compare
yanxinyi
force-pushed
the
xiyan/skip-shuffle-shared-guard
branch
from
September 29, 2026 15:50
d3d5369 to
e706f20
Compare
…huffle safety Adds runtime safety for the Flink runner's skipReshuffleForParDo optimization, which forwards input locally via reinterpretAsKeyedStream instead of a real HASH exchange, relying on the caller's assertion that the input already arrives correctly key-partitioned. When that assertion is violated, records can silently corrupt keyed state or crash the job with a cryptic internal KeyGroupRange exception. - SkipShufflePartitionGuard: a tested runtime guard used by the stateful-ParDo translator. Validates parallelism <= maxParallelism at graph-construction time, and per-record that each record actually arrived at the subtask a HASH exchange on its key would have routed it to (KeyGroupRangeAssignment#assignKeyToParallelOperator). On mismatch, either fails the job fast with a clear, actionable error (default), or -- via the new skipShuffleGuardDropMisalignedRecords option -- logs, counts (skipShufflePartitionGuardDroppedRecords metric), and drops just the offending record so the job keeps running. - FlinkSubtaskContextAware: a minimal opt-in interface letting any user DoFn learn its own (subtaskIndex, parallelism, maxParallelism), wired into DoFnOperator#open() (1.17 and 1.20). This lets downstream, domain-aware consumers (e.g. an application's own dead-letter-queue routing) build their own alignment check with domain-specific handling that this runner-internal guard has no way to provide itself. - Relaxes the graph-construction-time precondition from parallelism == maxParallelism to parallelism <= maxParallelism, matching the same looser contract FlinkSQL's StreamExecIntervalJoin already allows (source numShards == maxParallelism, job parallelism <= numShards). The runtime guard remains the safety net if that assumption doesn't actually hold. - Scoped to skipReshuffleForParDo only (no skipReshuffleForCoGroupByKey): CTC is currently the only consumer of this runner's skip-shuffle optimization, and CTC does not use CoGroupByKey, so this keeps the change minimal rather than carrying an unused option/guard path. - Adds ParDoSkipShuffleTest and DoFnOperatorTest coverage for the translator's guard behavior (fail-fast + drop mode) and the new FlinkSubtaskContextAware wiring. - Bumps version 2.45.58 -> 2.45.59. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yanxinyi
force-pushed
the
xiyan/skip-shuffle-shared-guard
branch
from
October 15, 2026 00:58
e706f20 to
459a2e4
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yananhao12
reviewed
Oct 15, 2026
yananhao12
left a comment
There was a problem hiding this comment.
Thanks for adding this — failing fast with an actionable message instead of the opaque Key group X is not in KeyGroupRange error is a clear improvement.
Three concerns inline:
skipShuffleGuardDropMisalignedRecordscan silently drop most of the traffic (e.g. after a rescale). My main ask: remove it or bound it.- The guard reads maxParallelism from its own vertex, which can differ from the stateful operator's (side inputs + unset maxP + restore).
- The drop-mode test would pass even if every record were dropped.
…d guard harness test - Remove skipShuffleGuardDropMisalignedRecords and the guard's drop path; misalignment is deterministic per key so dropping loses whole key ranges. The guard now only fails fast; consumers can route records themselves via FlinkSubtaskContextAware. - Fail translation when skipReshuffleForParDo is set without an explicit maxParallelism, so the guard and stateful operator agree on key groups. - Replace the weak drop-mode pipeline test with deterministic OneInputStreamOperatorTestHarness tests (p == maxP and p < maxP). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yananhao12
approved these changes
Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SkipShufflePartitionGuard, a runtime safety net for theskipReshuffleForParDooptimization (stateful ParDo), which forwards input locally via
reinterpretAsKeyedStreaminstead of a real HASH shuffle, relying on the caller's assertion that the input already
arrives correctly key-partitioned.
dropMisalignedRecordsmode to the guard: instead of always failing the job fast onthe first misaligned record, the guard can now log + count
(
skipShufflePartitionGuardDroppedRecordsmetric) + drop just the offending record and keepthe job running, controlled by a new
skipShuffleGuardDropMisalignedRecordspipeline option.FlinkSubtaskContextAware, a minimal opt-in interface that lets any userDoFnlearn itsown
(subtaskIndex, parallelism, maxParallelism)— wired intoDoFnOperator.open()(1.17 and1.20) — so downstream domain code (e.g. CTC's own guard
DoFn) can independently detectmisaligned records and route them to a domain-specific dead-letter sink, which this
runner-internal guard has no way to do itself.
parallelism == maxParallelismtoparallelism <= maxParallelism, matching whatreinterpretAsKeyedStreamactually requires.skipReshuffleForParDoonly — does not add or touchskipReshuffleForCoGroupByKey.CTC is currently the only consumer of this runner's skip-shuffle optimization, and CTC does not
use
CoGroupByKey, so this keeps the change minimal rather than carrying an option/guard pathnothing exercises yet. (An earlier version of this branch had added
CoGroupByKeysupportalongside this refactor; it was dropped before merge to keep the PR scoped to an actual
consumer's need — see "Notes for reviewers" below.)
Problem
skipReshuffleForParDoskips a real HASH shuffle before a keyed stateful ParDo, relying on thecaller's assertion that the input already arrives partitioned exactly as such a shuffle would
partition it. When that assertion is violated (e.g. an upstream source partitioning bug), the
record silently reaches a stateful operator whose keyed state does not own its key's key-group.
This either:
IllegalArgumentException: Key group X is not in KeyGroupRange{...}deep in the state backend (RocksDBKeyedStateBackend.setCurrentKey), orPrior to this PR, there was no runtime validation of this precondition at all, and no way to
recover from (rather than crash on) a misaligned record.
Solution
SkipShufflePartitionGuard.checkParallelismAligned(...): one-time, graph-construction-timecheck that
parallelism <= maxParallelism(required forreinterpretAsKeyedStreamto be a validsubstitute for a real HASH shuffle).
SkipShufflePartitionGuard.guard(...): a per-recordRichFlatMapFunction, inserted just beforereinterpretAsKeyedStreamin the stateful-ParDo translator, that recomputesKeyGroupRangeAssignment.assignKeyToParallelOperator(key, maxParallelism, parallelism)andcompares it to the record's actual subtask. On mismatch: fail-fast (default) or
log+count+drop (
dropMisalignedRecords=true).FlinkSubtaskContextAware: a narrow, generic hook (three ints, no fullRuntimeContextexposure) that any
DoFncan opt into, giving domain-specific consumers (like CTC) the sameruntime facts this guard uses, so they can build their own alignment check with
domain-specific handling (e.g. dead-lettering) that this runner-internal utility cannot provide.
Files changed
FlinkSubtaskContextAware.java(new)DoFnSkipShufflePartitionGuard.java(new)skipReshuffleForParDo, with fail-fast and drop modesDoFnOperator.java(1.17 and 1.20)setFlinkSubtaskContext(...)inopen()for anyDoFnimplementing the new interfaceFlinkPipelineOptions.javaskipShuffleGuardDropMisalignedRecordspipeline optionFlinkStreamingTransformTranslators.java<=ParDoSkipShuffleTest.java,DoFnOperatorTest.java(new/extended)FlinkSubtaskContextAwarewiringflink_java_pipeline_options.html,flink_python_pipeline_options.htmlgradle.properties,BeamModulePlugin.groovyTesting Done
:runners:flink:1.20:test— new tests pass (ParDoSkipShuffleTest6/6,DoFnOperatorTest#testFlinkSubtaskContextIsInjectedIntoOpenDoFn); pre-existing unrelatedJVM/environment failures (JDK 17 module-access) excluded
:runners:flink:1.20:compileJava/compileTestJavasucceed:runners:flink:spotlessJavaCheckpassesrunners:spark:3:validatesRunnerStreaming'sParDoLifecycleTestfailure in CI ispre-existing/environment-related, not caused by this diff (no changes touch
runners/spark;reproduced locally with a real pass once JDK 17
--add-opensflags were forced)Notes for reviewers
skipReshuffleForParDois explicitly enabled.skipReshuffleForCoGroupByKeywas deliberately not included in this PR. CTC is the onlycurrent consumer of this runner's skip-shuffle feature and doesn't use
CoGroupByKey; addingguard support for an option nothing exercises yet would be unvalidated, untested-in-production
scope creep. If/when a real
CoGroupByKeyconsumer needs this, it can be added as its own PRagainst
SkipShufflePartitionGuardat that time.linkedin-multiproduct/ctcPR [BEAM-2247] implement date functions apache/beam#3207 depends onFlinkSubtaskContextAwareto build a DLQ-routing guard on top of this.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.