Skip to content

[Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large.#3521

Merged
zhoujinsong merged 8 commits intoapache:masterfrom
wardlican:apache/amoro#3502
Apr 25, 2025
Merged

[Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large.#3521
zhoujinsong merged 8 commits intoapache:masterfrom
wardlican:apache/amoro#3502

Conversation

@wardlican
Copy link
Contributor

@wardlican wardlican commented Apr 16, 2025

Why are the changes needed?

Close #3502.

Brief change log

  1. In the close method, in addition to setting the worker thread exit flag, the worker thread is also gracefully exited.
  2. If the worker thread does not exit, it may cause the problem of repeatedly creating multiple worker threads in some scenarios, which may cause OOM problems due to the existence of multiple worker threads.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

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

@github-actions github-actions bot added the module:ams-optimizer AMS optimizer module label Apr 16, 2025
@Aireed Aireed force-pushed the apache/amoro#3502 branch from 47bc6c9 to 8cbc056 Compare April 16, 2025 02:30
@Aireed
Copy link
Contributor

Aireed commented Apr 16, 2025

Can you change the information according to the PR specification?You can refer to #3432

@zhoujinsong zhoujinsong changed the title [Bug]: The flink optimizer often triggers OOM exceptions. In fact, th… [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. Apr 16, 2025
@zhoujinsong
Copy link
Contributor

Thanks for the contribution!

It seems that some check style error you need to fix, you can run mvn spotless:apply in the root directory to do that.

Copy link
Contributor

@xxubai xxubai left a comment

Choose a reason for hiding this comment

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

Can you explain this change. Why make the thread be a daemon thread

wardli added 3 commits April 16, 2025 19:14
# Conflicts:
#	amoro-optimizer/amoro-optimizer-flink/src/main/java/org/apache/amoro/optimizer/flink/FlinkExecutor.java
@wardlican
Copy link
Contributor Author

Can you explain this change. Why make the thread be a daemon thread

This design not only ensures graceful exit during normal shutdown, but also provides an additional safety guarantee through the characteristics of daemon threads to prevent threads from preventing JVM exit under abnormal circumstances.

@xxubai
Copy link
Contributor

xxubai commented Apr 17, 2025

This design not only ensures graceful exit during normal shutdown, but also provides an additional safety guarantee through the characteristics of daemon threads to prevent threads from preventing JVM exit under abnormal circumstances.

For data process task, maybe user thread is a better choice? WDYT @zhoujinsong

@zhoujinsong
Copy link
Contributor

For data process task, maybe user thread is a better choice? WDYT @zhoujinsong

Setting the thread to daemon looks good to me. The process handling thread should not protect the TM from closing.

Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhoujinsong zhoujinsong merged commit 6a4f667 into apache:master Apr 25, 2025
3 checks passed
Jzjsnow pushed a commit to Jzjsnow/amoro that referenced this pull request Aug 20, 2025
…e files processed by each task are not large. (apache#3521)

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. apache#3502

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. apache#3502

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. apache#3502

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. apache#3502

---------

Co-authored-by: wardli <wardli@tencent.com>
Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
(cherry picked from commit 6a4f667)
zhoujinsong added a commit that referenced this pull request Aug 25, 2025
…e files processed by each task are not large. (#3521)

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. #3502

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. #3502

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. #3502

* [Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large. #3502

---------

Co-authored-by: wardli <wardli@tencent.com>
Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
(cherry picked from commit 6a4f667)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-optimizer AMS optimizer module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: The flink optimizer often triggers OOM exceptions. In fact, the files processed by each task are not large.

4 participants