Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[FLINK-36067][runtime] Support optimize stream graph based on input info. #25790

Merged
merged 3 commits into from
Dec 26, 2024
Merged

[FLINK-36067][runtime] Support optimize stream graph based on input info. #25790

merged 3 commits into from
Dec 26, 2024

Conversation

JunRuiLee
Copy link
Contributor

What is the purpose of the change

[FLINK-36067][runtime] Support optimize stream graph based on input info.

Brief change log

  • Introduce StreamGraphOptimizer and StreamGraphOptimizationStrategy
  • Support optimize stream graph based on input info.

Verifying this change

This change added tests.

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

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
  • The serializers: (yes / no / don't know)
  • The runtime per-record code paths (performance sensitive): (yes / no / don't know)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know)
  • The S3 file system connector: (yes / no / don't know)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@flinkbot
Copy link
Collaborator

flinkbot commented Dec 12, 2024 •
edited
Loading

CI report:

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

@JunRuiLee JunRuiLee force-pushed the FLINK-36067-1212 branch 3 times, most recently from 9d3e01f to 9b2cc2a Compare December 12, 2024 11:03
@JunRuiLee JunRuiLee requested a review from zhuzhurk December 12, 2024 14:25
davidradl
davidradl reviewed Dec 12, 2024
View reviewed changes
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/IntermediateResult.java Outdated
@@ -207,6 +210,10 @@ public boolean isForward() {
return intermediateDataSet.isForward();
}

public boolean isEveryConsumerConsumeAllSubPartitions() {
Copy link
Contributor

@davidradl davidradl Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I am confused by why this method is checking broadcasts - how does this relate to downstream consumers being able to consume the subpartitions

Copy link
Contributor Author

@JunRuiLee JunRuiLee Dec 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, it was indeed confusing before to have both broadcast and EveryConsumerConsumeAllSubPartitions present at the same time.

Let me explain the reason for introducing these changes: Broadcast was used to represent two specific modes for reading and writing. Writing required that a single subpartition contains all data, and reading corresponded to consuming that single subpartition.

In my opinion, broadcasting requires that the downstream consumes all produced data. The optimization is that the upstream could write all data to a single subpartition, and the downstream could consume it. These are different concepts.

So, in this PR, I introduced singleSubpartitionContainsAllData to replace the original optimization concept. The broadcast concept will change to mean the downstream consumes all produced data.

zhuzhurk
zhuzhurk reviewed Dec 22, 2024
View reviewed changes
Copy link
Contributor

@zhuzhurk zhuzhurk left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for creating this PR! @JunRuiLee
I have a few comments. PTAL.

...untime/src/main/java/org/apache/flink/runtime/scheduler/adaptivebatch/OperatorsFinished.java Outdated Show resolved Hide resolved
...untime/src/main/java/org/apache/flink/runtime/scheduler/adaptivebatch/OperatorsFinished.java Outdated Show resolved Hide resolved
...n/java/org/apache/flink/runtime/scheduler/adaptivebatch/StreamGraphOptimizationStrategy.java Outdated Show resolved Hide resolved
...n/java/org/apache/flink/runtime/scheduler/adaptivebatch/StreamGraphOptimizationStrategy.java Outdated Show resolved Hide resolved
...ime/src/main/java/org/apache/flink/runtime/scheduler/adaptivebatch/StreamGraphOptimizer.java Outdated Show resolved Hide resolved
...e/src/main/java/org/apache/flink/runtime/scheduler/adaptivebatch/AdaptiveBatchScheduler.java Show resolved Hide resolved
...c/main/java/org/apache/flink/runtime/scheduler/adaptivebatch/AllToAllBlockingResultInfo.java Show resolved Hide resolved
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/IntermediateResultInfo.java Outdated Show resolved Hide resolved
...c/main/java/org/apache/flink/runtime/scheduler/adaptivebatch/AbstractBlockingResultInfo.java Outdated Show resolved Hide resolved
...e/src/main/java/org/apache/flink/runtime/scheduler/adaptivebatch/AdaptiveBatchScheduler.java Show resolved Hide resolved
@JunRuiLee JunRuiLee force-pushed the FLINK-36067-1212 branch 8 times, most recently from ff6840e to 95f7597 Compare December 24, 2024 16:18
@JunRuiLee JunRuiLee force-pushed the FLINK-36067-1212 branch 2 times, most recently from bd01f20 to 79262b4 Compare December 26, 2024 02:52
zhuzhurk
zhuzhurk approved these changes Dec 26, 2024
View reviewed changes
Copy link
Contributor

@zhuzhurk zhuzhurk left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all the comments. @JunRuiLee
LGTM.

@JunRuiLee JunRuiLee merged commit 95f9a16 into apache:master Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidradl davidradl davidradl left review comments

@zhuzhurk zhuzhurk zhuzhurk approved these changes

Assignees
No one assigned
Labels
component=Runtime/Coordination
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JunRuiLee @flinkbot @zhuzhurk @davidradl