Skip to content

[FLINK-40269][runtime] Fix channel state assignment for duplicate connections - #28856

Merged
1996fanrui merged 2 commits into
apache:masterfrom
1996fanrui:FLINK-40269-unaligned-same-upstream-rescale
Aug 1, 2026
Merged

[FLINK-40269][runtime] Fix channel state assignment for duplicate connections#28856
1996fanrui merged 2 commits into
apache:masterfrom
1996fanrui:FLINK-40269-unaligned-same-upstream-rescale

Conversation

@1996fanrui

Copy link
Copy Markdown
Member

What is the purpose of the change

This pull request fixes channel state assignment for unaligned checkpoint recovery when a downstream job vertex has multiple input gates connected to the same upstream job vertex.

Previously, TaskStateAssignment looked up connected assignments by the upstream/downstream TaskStateAssignment instance. If multiple edges connect the same pair of job vertices, this can select the first matching edge and use the wrong input gate or result partition mapping.

Brief change log

  • Use IntermediateDataSetID to resolve the corresponding input gate and result partition during channel state assignment.
  • Add an ITCase covering unaligned checkpoint rescaling with duplicate same-upstream inputs.
  • Add a unit test covering duplicate job vertex connections with different channel state mappers.

Verifying this change

This change added tests and can be verified as follows:

  • ./mvnw -pl flink-runtime -Dtest=StateAssignmentOperationTest -Djdk11 -Pjava11-target test
  • ./mvnw -pl flink-tests -Dtest=UnalignedCheckpointRescaleSameUpstreamITCase -Dsurefire.failIfNoSpecifiedTests=false -Djdk11 -Pjava11-target -DtrimStackTrace=false surefire:test@integration-tests

Does this pull request potentially affect one of the following parts:

  • Dependencies: no
  • Public API: no
  • Serializers: no
  • Runtime per-record code paths: no
  • Deployment or recovery: yes, checkpoint recovery
  • S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no

@1996fanrui
1996fanrui marked this pull request as ready for review July 31, 2026 12:55
@flinkbot

flinkbot commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@rkhachatryan rkhachatryan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

LGTM, I have one minor suggestion to reduce complexity, PTAL.

Comment on lines +269 to +275
(gateIndex, assignment, recompute) -> {
int assignmentIndex =
getAssignmentIndex(
assignment.getDownstreamAssignments(), this);
assignment.findResultPartitionIndex(
executionJobVertex
.getInputs()
.get(gateIndex)
.getId());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we simplify this with something like


  private SubtasksRescaleMapping getOutputMapping(
          IntermediateDataSetID resultId, boolean recompute) {
      return getOutputMapping(findResultPartitionIndex(resultId), recompute);
  }

  private SubtasksRescaleMapping getInputMapping(
          IntermediateDataSetID resultId, boolean recompute) {
      return getInputMapping(findInputGateIndex(resultId), recompute);
  }


  // getSubtaskState (input side)
  (gateIndex, assignment, recompute) ->
          assignment.getOutputMapping(
                  executionJobVertex.getInputs().get(gateIndex).getId(), recompute)

  // computeOutputRescalingDescriptor (output side)
  (partitionIndex, downstreamAssignment, recompute) ->
          downstreamAssignment.getInputMapping(
                  executionJobVertex.getProducedDataSets()[partitionIndex].getId(), recompute)

?

@rkhachatryan rkhachatryan Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we can get rid of lambda and interface completely:
#28860 (26be3b1)
WDYT?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rkhachatryan , thanks for the quick review!

I have merged 26be3b1 into the second commit, and only fixed some checkstyle issues.

FYI: and marked you as the co-author.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

(I'm not the co-author of the fix though)

…nections

Co-authored-by: Roman Khachatryan <khachatryan.roman@gmail.com>
@1996fanrui
1996fanrui force-pushed the FLINK-40269-unaligned-same-upstream-rescale branch from 039cb7a to 8652d59 Compare July 31, 2026 20:36

@wenshao wenshao left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— qwen3.8-max-preview via Qwen Code /review (v0.21.2)

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Aug 1, 2026
@1996fanrui
1996fanrui merged commit 9f867da into apache:master Aug 1, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— qwen3.8-max-preview via Qwen Code /review (v0.21.2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants