Repository navigation
[FLINK-40780][runtime] Make FORWARD-to-REBALANCE downgrade on parallelism mismatch configurable #29272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-40780][runtime] Make FORWARD-to-REBALANCE downgrade on parallelism mismatch configurable #29272
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,8 @@ | |||||
| import org.apache.flink.configuration.CheckpointingOptions; | ||||||
| import org.apache.flink.configuration.Configuration; | ||||||
| import org.apache.flink.configuration.NettyShuffleEnvironmentOptions; | ||||||
| import org.apache.flink.configuration.PipelineOptions; | ||||||
| import org.apache.flink.configuration.PipelineOptions.ForwardEdgeParallelismMismatchMode; | ||||||
| import org.apache.flink.configuration.TaskManagerOptions; | ||||||
| import org.apache.flink.core.execution.RecoveryClaimMode; | ||||||
| import org.apache.flink.core.fs.AutoCloseableRegistry; | ||||||
|
|
@@ -2051,18 +2053,60 @@ private static void replaceForwardPartitionerIfConsumerParallelismDoesNotMatch( | |||||
| // The number of subpartitions isn't the consumer parallelism on a POINTWISE edge, so the | ||||||
| // actual consumer parallelism is passed through the deployment descriptor. | ||||||
| final int consumerParallelism = environment.getWriterConsumerParallelism(outputIndex); | ||||||
| if (streamOutput.getPartitioner() instanceof ForwardPartitioner | ||||||
| if (!(streamOutput.getPartitioner() instanceof ForwardPartitioner) | ||||||
| // An undecided consumer parallelism is not a mismatch. | ||||||
| && consumerParallelism | ||||||
| != ResultPartitionDeploymentDescriptor.UNKNOWN_CONSUMER_PARALLELISM | ||||||
| && consumerParallelism != producerParallelism) { | ||||||
| LOG.debug( | ||||||
| "Replacing forward partitioner with rebalance for {} " | ||||||
| + "(producer parallelism {} != consumer parallelism {}).", | ||||||
| environment.getTaskInfo().getTaskNameWithSubtasks(), | ||||||
| producerParallelism, | ||||||
| consumerParallelism); | ||||||
| streamOutput.setPartitioner(new RebalancePartitioner<>()); | ||||||
| || consumerParallelism | ||||||
| == ResultPartitionDeploymentDescriptor.UNKNOWN_CONSUMER_PARALLELISM | ||||||
| || consumerParallelism == producerParallelism) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wdyt about
Suggested change
maybe we can keep the forward partitioner in this case as well, since each producer will still forward to 1 consumer?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer each mode to mean exactly one behavior:
Special-casing downscale would make REBALANCE silently behave like |
||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| final ForwardEdgeParallelismMismatchMode mode = | ||||||
| environment | ||||||
| .getJobConfiguration() | ||||||
| .get(PipelineOptions.FORWARD_EDGE_PARALLELISM_MISMATCH_MODE); | ||||||
| final String taskNameWithSubtasks = environment.getTaskInfo().getTaskNameWithSubtasks(); | ||||||
| switch (mode) { | ||||||
| case REBALANCE: | ||||||
| LOG.warn( | ||||||
| "Replacing forward partitioner with rebalance for {} " | ||||||
| + "(producer parallelism {} != consumer parallelism {}). This may " | ||||||
| + "reorder records and produce incorrect results for order-sensitive " | ||||||
| + "(changelog) streams. Set '{}' to {} or {} to avoid it.", | ||||||
| taskNameWithSubtasks, | ||||||
| producerParallelism, | ||||||
| consumerParallelism, | ||||||
| PipelineOptions.FORWARD_EDGE_PARALLELISM_MISMATCH_MODE.key(), | ||||||
| ForwardEdgeParallelismMismatchMode.FAIL, | ||||||
| ForwardEdgeParallelismMismatchMode.KEEP_FORWARD); | ||||||
| streamOutput.setPartitioner(new RebalancePartitioner<>()); | ||||||
| break; | ||||||
| case KEEP_FORWARD: | ||||||
| LOG.warn( | ||||||
| "Keeping forward partitioner for {} despite a parallelism mismatch " | ||||||
| + "(producer parallelism {} != consumer parallelism {}). Record order " | ||||||
| + "is preserved but records are funneled to a single consumer subtask, " | ||||||
| + "leaving the remaining consumer subtasks idle.", | ||||||
| taskNameWithSubtasks, | ||||||
| producerParallelism, | ||||||
| consumerParallelism); | ||||||
| break; | ||||||
| case FAIL: | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why do we need the fail mode?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FAIL is for setups where a parallelism mismatch is never expected, so it's surfaced immediately instead of being handled silently. |
||||||
| throw new FlinkRuntimeException( | ||||||
| String.format( | ||||||
| "Forward partitioning cannot be preserved across a parallelism change " | ||||||
| + "for %s (producer parallelism %d != consumer parallelism %d). " | ||||||
| + "Silently downgrading a FORWARD edge to a redistributing " | ||||||
| + "exchange can reorder records and corrupt order-sensitive " | ||||||
| + "(changelog) results. Set %s to %s or %s to allow it.", | ||||||
| taskNameWithSubtasks, | ||||||
| producerParallelism, | ||||||
| consumerParallelism, | ||||||
| PipelineOptions.FORWARD_EDGE_PARALLELISM_MISMATCH_MODE.key(), | ||||||
| ForwardEdgeParallelismMismatchMode.REBALANCE, | ||||||
| ForwardEdgeParallelismMismatchMode.KEEP_FORWARD)); | ||||||
| default: | ||||||
| throw new IllegalStateException("Unhandled mode: " + mode); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC
REBALANCEbreaks changelog mode - should it be the default?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. REBALANCE stays the default to keep the existing behavior, so jobs relying on it today (e.g. append-only jobs rescaled by the AdaptiveScheduler) aren't affected. Changelog jobs can opt into FAIL or KEEP_FORWARD.