fix(common): Close log writer output stream on append failure by fhan688 · Pull Request #18909 · apache/hudi

fhan688 · 2026-06-03T10:35:50Z

Describe the issue this Pull Request addresses

HoodieLogFormatWriter#appendBlocks can leave the underlying FSDataOutputStream open when an exception is thrown after the output stream has already been opened, for example while writing log block bytes. In this
failure path, the writer does not release the stream/lease until a later close path, which may not be reached by callers.

This PR ensures the output stream is closed on append failure while preserving the original failure as the primary exception.

Summary and Changelog

This PR improves failure-path resource cleanup in HoodieLogFormatWriter.

Changes:

Close the underlying output stream when appendBlocks fails after opening the stream.
Preserve the original append exception type and message instead of wrapping it.
Add close failures as suppressed exceptions so they do not hide the append failure.
Ensure closeStream still closes the output stream even when sync() fails.
Add unit tests covering:
- append write failure closes the output stream;
- append write failure is preserved when close also fails;
- close still closes the output stream when sync() fails.

No code was copied.

Impact

No public API change.

Successful append behavior is unchanged. This PR does not restore append-time sync()/hsync, so the existing append performance behavior remains intact.

The behavioral change is limited to failure handling:

failed appends now release the writer output stream immediately;
close failures are attached as suppressed exceptions instead of replacing the original append failure.

Risk Level

low

The change is scoped to error handling and resource cleanup in HoodieLogFormatWriter. The successful write path remains functionally unchanged.

Verification:

mvn -pl hudi-hadoop-common -am -DskipITs -Dcheckstyle.skip=true -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false -Dtest=TestHoodieLogFormatWriter test

Result:
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0

Documentation Update

none

No new config, public API, or user-facing feature is added.

Contributor's checklist

Read through contributor's guide (https://hudi.apache.org/contribute/how-to-contribute)
Enough context is provided in the sections above
Adequate tests were added if applicable

hudi-agent

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for the contribution! This PR adds proper resource cleanup when HoodieLogFormatWriter.appendBlocks fails after opening the output stream, and ensures closeStream still closes the underlying stream when sync() throws — preserving the original exception and attaching close failures as suppressed exceptions. The control flow in closeStream correctly handles all four combinations of sync/close success/failure, and closeOutputStream uses a try-finally so writer state (output=null, closed=true) is updated even when output.close() throws. The behavioral change (writer becomes unusable after a failed append, since closed=true makes subsequent getCurrentSize() throw IllegalStateException) appears intentional and is asserted in the new tests; no callers in this repo rely on retrying on the same writer after a failed append. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. One naming nit on a newly added helper method; everything else is clean.

cc @yihua

hudi-agent

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for the iteration here. The original fix (closing the output stream on append failure) is preserved intact — closeStream, closeOutputStream, and closeOutputStreamOnAppendFailure are all still present and correctly handle the sync/close paths. However, the PR has expanded considerably beyond its original "fix log writer close on append failure" scope: it now also bundles a large Lombok refactor, the HoodieLogFormat.Writer interface → abstract class conversion, removal of WriterBuilder, builder method renames (withFileId → withLogFileId, onParentPath → withParentPath), and HoodieCleanStat builder method renames/constructor removal. These are binary/source-incompatible changes for any downstream consumers — it might be worth splitting them into a focused follow-up so reviewers can evaluate them independently from the original bug fix. The prior nit about renaming throwAsIOExceptionOrRuntimeException doesn't appear to be addressed but it's a nit and not blocking. One small inline question about the boolean → Boolean change in HoodieReaderContext. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here.

voonhous · 2026-06-04T04:11:20Z

Heyo @fhan688, we recently started cleaning up our builder patterns with Lombok to reduce boilerplate code.

Error:  /home/runner/work/hudi/hudi/hudi-hadoop-common/src/test/java/org/apache/hudi/common/table/log/TestHoodieLogFormatWriter.java:[99,51] cannot find symbol

...

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.14.1:testCompile (default-testCompile) on project hudi-hadoop-common: Compilation failure
Error:  /home/runner/work/hudi/hudi/hudi-hadoop-common/src/test/java/org/apache/hudi/common/table/log/TestHoodieLogFormatWriter.java:[99,51] cannot find symbol
Error:    symbol:   method newWriterBuilder()
Error:    location: interface org.apache.hudi.common.table.log.HoodieLogFormat
Error:  -> [Help 1]

The error you are seeing is a builder pattern migration introduce in:

https://github.com/apache/hudi/pull/17785/changes#diff-df46d638afac77124f4b075a88ae64f7a74088fc514cd2ea33328e9687b0eaeeR55-R66

Just a headsup.

fhan688 · 2026-06-04T13:57:03Z

Heyo @fhan688, we recently started cleaning up our builder patterns with Lombok to reduce boilerplate code.
Error:  /home/runner/work/hudi/hudi/hudi-hadoop-common/src/test/java/org/apache/hudi/common/table/log/TestHoodieLogFormatWriter.java:[99,51] cannot find symbol

...

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.14.1:testCompile (default-testCompile) on project hudi-hadoop-common: Compilation failure
Error:  /home/runner/work/hudi/hudi/hudi-hadoop-common/src/test/java/org/apache/hudi/common/table/log/TestHoodieLogFormatWriter.java:[99,51] cannot find symbol
Error:    symbol:   method newWriterBuilder()
Error:    location: interface org.apache.hudi.common.table.log.HoodieLogFormat
Error:  -> [Help 1]
The error you are seeing is a builder pattern migration introduce in:

https://github.com/apache/hudi/pull/17785/changes#diff-df46d638afac77124f4b075a88ae64f7a74088fc514cd2ea33328e9687b0eaeeR55-R66

Just a headsup.

thanks! I'll fix this later.

…nOrRuntimeException to rethrow

danny0405 · 2026-06-05T03:11:15Z

+    }
+  }
+
+  private void rethrow(Exception exception) throws IOException {


is this right? the method does not always throw IOException now?

is this right? the method does not always throw IOException now?

Good point. I used Exception to simplify the catch path, but made rethrow handle generic checked exceptions safely by preserving IOException and RuntimeException, and wrapping any other checked exception as IOException

codecov-commenter · 2026-06-05T07:38:42Z

Codecov Report

❌ Patch coverage is 80.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.82%. Comparing base (163a15a) to head (4116567).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
...e/hudi/common/table/log/HoodieLogFormatWriter.java	80.00%	10 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18909      +/-   ##
============================================
+ Coverage     68.77%   68.82%   +0.04%     
- Complexity    29087    29097      +10     
============================================
  Files          2517     2517              
  Lines        139822   139842      +20     
  Branches      17209    17207       -2     
============================================
+ Hits          96169    96243      +74     
+ Misses        35880    35820      -60     
- Partials       7773     7779       +6

Flag	Coverage Δ
common-and-other-modules	`44.50% <56.66%> (+0.25%)`	⬆️
hadoop-mr-java-client	`44.75% <56.66%> (+<0.01%)`	⬆️
spark-client-hadoop-common	`48.06% <80.00%> (+0.01%)`	⬆️
spark-java-tests	`49.31% <68.33%> (+<0.01%)`	⬆️
spark-scala-tests	`45.21% <56.66%> (-0.01%)`	⬇️
utilities	`37.30% <56.66%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...e/hudi/common/table/log/HoodieLogFormatWriter.java	`80.91% <80.00%> (-0.28%)`	⬇️

... and 46 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hudi-bot · 2026-06-05T07:54:19Z

CI report:

4116567 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

hudi-agent

🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! The PR closes the underlying FSDataOutputStream when appendBlocks fails after the stream has been opened, preserving the original exception while attaching close failures as suppressed. The closeStream refactor also correctly handles the case where sync() fails by still attempting to close the output stream. No new critical correctness issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

fix(common): Close log writer output stream on append failure

5904445

hudi-agent reviewed Jun 3, 2026

View reviewed changes

Comment thread hudi-hadoop-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java Outdated

Merge branch 'master' into fix-not-close-log-file-when-exception

1c4568f

github-actions Bot added the size:L PR with lines of changes in (300, 1000] label Jun 3, 2026

hudi-agent reviewed Jun 3, 2026

View reviewed changes

cshuo reviewed Jun 4, 2026

View reviewed changes

Comment thread hudi-hadoop-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java Outdated

fix(common): fix newWriterBuilder errors and rename throwAsIOExceptio…

7864c76

…nOrRuntimeException to rethrow

danny0405 reviewed Jun 5, 2026

View reviewed changes

fix(common): optimize impl in rethrow()

4116567

hudi-agent reviewed Jun 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(common): Close log writer output stream on append failure#18909

fix(common): Close log writer output stream on append failure#18909
fhan688 wants to merge 4 commits into
apache:masterfrom
fhan688:fix-not-close-log-file-when-exception

fhan688 commented Jun 3, 2026

Uh oh!

hudi-agent left a comment

Uh oh!

Uh oh!

hudi-agent left a comment

Uh oh!

voonhous commented Jun 4, 2026

Uh oh!

Uh oh!

fhan688 commented Jun 4, 2026

Uh oh!

danny0405 Jun 5, 2026

Uh oh!

fhan688 Jun 5, 2026

Uh oh!

codecov-commenter commented Jun 5, 2026

Uh oh!

hudi-bot commented Jun 5, 2026

Uh oh!

hudi-agent left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

fhan688 commented Jun 3, 2026

Describe the issue this Pull Request addresses

Summary and Changelog

Impact

Risk Level

Documentation Update

Contributor's checklist

Uh oh!

hudi-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hudi-agent left a comment

Choose a reason for hiding this comment

Uh oh!

voonhous commented Jun 4, 2026

Uh oh!

Uh oh!

fhan688 commented Jun 4, 2026

Uh oh!

danny0405 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

fhan688 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jun 5, 2026

Codecov Report

Uh oh!

hudi-bot commented Jun 5, 2026

CI report:

Uh oh!

hudi-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants