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

[HUDI-8794] Make sure the log file has been closed and then remove the shutdown hook #12542

Merged
merged 3 commits into from
Jan 3, 2025
+5 −6
Merged

[HUDI-8794] Make sure the log file has been closed and then remove the shutdown hook #12542

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
467cd95
fix: make sure log file has been closed and then remove the shutdown …
TheR1sing3un Dec 26, 2024
b56e501
rerun
TheR1sing3un Jan 2, 2025
e1c142c
rerun
TheR1sing3un Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions hudi-hadoop-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,11 @@ private void createNewFile() throws IOException {

@Override
public void close() throws IOException {
closeStream();
// remove the shutdown hook after closing the stream to avoid memory leaks
if (null != shutdownThread) {
Runtime.getRuntime().removeShutdownHook(shutdownThread);
}
closeStream();
}

private void closeStream() throws IOException {
Expand Down Expand Up @@ -275,12 +276,10 @@ private void addShutDownHook() {
shutdownThread = new Thread() {
public void run() {
try {
LOG.warn("running logformatwriter hook");
if (output != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the check output != null ?

Copy link
Member Author

@TheR1sing3un TheR1sing3un Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why remove the check output != null ?

closeStream method already has the same logic:
image

closeStream();
}
LOG.warn("running HoodieLogFormatWriter shutdown hook to close output stream for log file: {}", logFile);
closeStream();
} catch (Exception e) {
LOG.warn(String.format("unable to close output stream for log file %s", logFile), e);
LOG.warn("unable to close output stream for log file: {}", logFile, e);
// fail silently for any sort of exception
}
}
Expand Down
Loading