This is an automated email from the ASF dual-hosted git repository. voonhous pushed a commit to branch release-1.2.1 in repository https://gitbox.apache.org/repos/asf/hudi.git
commit b85f1cc3e14a3ff89f843b784d4346a44e5e69c2 Author: chaoyang <[email protected]> AuthorDate: Sun May 10 11:03:19 2026 +0800 perf: Reduce unnecessary FSDataOutputStream#hsync to enhance append performance (#17517) * perf: Reduce unnecessary `FSDataOutputStream#hsync` to enhance append performance 1. Reduce unnecessary `FSDataOutputStream#hsync` to enhance append performance Signed-off-by: TheR1sing3un <[email protected]> * feat: flush behavior compatible with the block append mode 1. flush behavior compatible with the block append mode Signed-off-by: TheR1sing3un <[email protected]> * fixup: address review - drop syncDuringFlush, expose explicit sync() Following @danny0405's suggestion in the PR review, ensure only commit-level visibility on the production path: - Remove the `withSyncDuringFlush` builder option and the `flush(boolean)` overload on HoodieLogFormatWriter; the production path no longer flushes or hsyncs at appendBlocks. - Expose `Writer#sync()` (flush + hsync) as an explicit API for tests that assert per-append visibility on the underlying file system. - closeStream still calls sync() once before close so a closed writer guarantees data is persisted to DataNodes. - Update tests that previously relied on `withSyncDuringFlush(true)` to call `writer.sync()` explicitly before per-append FileStatus size assertions, and rename the related assertion message to drop the misleading "auto-flushed" wording. --------- Signed-off-by: TheR1sing3un <[email protected]> (cherry picked from commit 1fffd70db1cb7b7e0441a09842b0d152dfaafa2e) --- .../apache/hudi/common/table/log/HoodieLogFormat.java | 11 +++++++++++ .../hudi/common/table/log/HoodieLogFormatWriter.java | 17 +++++++++++------ .../hudi/common/functional/TestHoodieLogFormat.java | 9 ++++++--- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormat.java b/hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormat.java index a1103705c3c2..2c536ec07722 100644 --- a/hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormat.java +++ b/hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormat.java @@ -85,6 +85,17 @@ public interface HoodieLogFormat { AppendResult appendBlocks(List<HoodieLogBlock> blocks) throws IOException, InterruptedException; long getCurrentSize() throws IOException; + + /** + * Force previously appended blocks to durable storage so that downstream + * readers can observe them before this writer is closed. + * + * <p>Production code paths typically rely on {@link #close()} for + * commit-level visibility and do not need to call this. It is exposed + * mainly for tests that assert per-append visibility on the underlying + * file system. + */ + void sync() throws IOException; } /** diff --git a/hudi-hadoop-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java b/hudi-hadoop-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java index 23a69699a43e..1d6acc8d85ef 100644 --- a/hudi-hadoop-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java +++ b/hudi-hadoop-common/src/main/java/org/apache/hudi/common/table/log/HoodieLogFormatWriter.java @@ -179,8 +179,10 @@ public class HoodieLogFormatWriter implements HoodieLogFormat.Writer { } sizeWritten += outputStream.size() - startSize; } - // Flush all blocks to disk - flush(); + // No flush/hsync here: append-time visibility is not part of the contract. + // Downstream readers only need commit-level visibility, which is provided + // when the writer is closed (see closeStream) or when callers explicitly + // invoke sync(). AppendResult result = new AppendResult(logFile, startPos, sizeWritten); // roll over if size is past the threshold @@ -236,20 +238,23 @@ public class HoodieLogFormatWriter implements HoodieLogFormat.Writer { private void closeStream() throws IOException { if (output != null) { - flush(); + // Persist all buffered data to DataNodes before closing so downstream + // readers can observe a fully-written log file at commit-level visibility. + sync(); output.close(); output = null; closed = true; } } - private void flush() throws IOException { + @Override + public void sync() throws IOException { if (output == null) { return; // Presume closed } output.flush(); - // NOTE : the following API call makes sure that the data is flushed to disk on DataNodes (akin to POSIX fsync()) - // See more details here : https://issues.apache.org/jira/browse/HDFS-744 + // NOTE: the following API call makes sure that the data is flushed to disk on DataNodes (akin to POSIX fsync()) + // See more details here: https://issues.apache.org/jira/browse/HDFS-744 output.hsync(); } diff --git a/hudi-hadoop-common/src/test/java/org/apache/hudi/common/functional/TestHoodieLogFormat.java b/hudi-hadoop-common/src/test/java/org/apache/hudi/common/functional/TestHoodieLogFormat.java index cde861f8c5f2..7e3ab4ebbbb8 100755 --- a/hudi-hadoop-common/src/test/java/org/apache/hudi/common/functional/TestHoodieLogFormat.java +++ b/hudi-hadoop-common/src/test/java/org/apache/hudi/common/functional/TestHoodieLogFormat.java @@ -234,8 +234,9 @@ public class TestHoodieLogFormat extends HoodieCommonTestHarness { long size = writer.getCurrentSize(); assertTrue(size > 0, "We just wrote a block - size should be > 0"); + writer.sync(); assertEquals(size, storage.getPathInfo(writer.getLogFile().getPath()).getLength(), - "Write should be auto-flushed. The size reported by FileStatus and the writer should match"); + "After explicit sync, FileStatus length should match the writer's reported size"); assertEquals(size, result.size()); assertEquals(writer.getLogFile(), result.logFile()); assertEquals(0, result.offset()); @@ -376,8 +377,9 @@ public class TestHoodieLogFormat extends HoodieCommonTestHarness { writer.appendBlock(dataBlock); long size2 = writer.getCurrentSize(); assertTrue(size2 > size1, "We just wrote a new block - size2 should be > size1"); + writer.sync(); assertEquals(size2, storage.getPathInfo(writer.getLogFile().getPath()).getLength(), - "Write should be auto-flushed. The size reported by FileStatus and the writer should match"); + "After explicit sync, FileStatus length should match the writer's reported size"); writer.close(); // Close and Open again and append 100 more records @@ -394,8 +396,9 @@ public class TestHoodieLogFormat extends HoodieCommonTestHarness { writer.appendBlock(dataBlock); long size3 = writer.getCurrentSize(); assertTrue(size3 > size2, "We just wrote a new block - size3 should be > size2"); + writer.sync(); assertEquals(size3, storage.getPathInfo(writer.getLogFile().getPath()).getLength(), - "Write should be auto-flushed. The size reported by FileStatus and the writer should match"); + "After explicit sync, FileStatus length should match the writer's reported size"); writer.close(); // Cannot get the current size after closing the log
