-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-37955] Don't load all records into main memory to avoid OOM #26888
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
Conversation
Originally, I fixed the issue in Flink 1.19 by replacing List with Iterator. While porting the fix to 2.0, I discovered that most of the code was extracted to be re-used in async versions of the operators. However, without some deeper rework, I don't see how my change can be used in async way; because async iterator requires callbacks to be passed in. So I'm inclined to have two different versions for sync (iterator) and async (list) for now. WDYT @xuyangzhong, @pnowojski ? |
...er/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/common/JoinTestPrograms.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/MiscSemanticTests.java
Outdated
Show resolved
Hide resolved
284575d
to
d936c73
Compare
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.
LGTM assuming green build
d936c73
to
fa4b8a2
Compare
@flinkbot run azure |
1 similar comment
@flinkbot run azure |
This failure seems to be unrelated:
Re-running the CI. |
The failure is unrelated: https://issues.apache.org/jira/browse/FLINK-35012 . |
We've discussed this offline with @pnowojski and @Zakelly and decided to have separate versions of the code for now |
Replace
List
of matched records withIterator
to avoid OOM errors inStreamingJoinOperator
and the like.Currently, PR targets only the sync operator version - see the discussion below.