This is an automated email from the ASF dual-hosted git repository.
danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/master by this push:
new 1fffd70db1cb perf: Reduce unnecessary FSDataOutputStream#hsync to
enhance append performance (#17517)
1fffd70db1cb is described below
commit 1fffd70db1cb7b7e0441a09842b0d152dfaafa2e
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]>
---
.../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
