This is an automated email from the ASF dual-hosted git repository.
smengcl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new cf30cd78257 HDDS-16327. Read the block file length once per WriteChunk
instead of three fstat calls (#11150)
cf30cd78257 is described below
commit cf30cd7825791592a640d7bee0437fd329fa9648
Author: KUAN-HAO HUANG <[email protected]>
AuthorDate: Fri Oct 9 10:31:32 2026 +0800
HDDS-16327. Read the block file length once per WriteChunk instead of three
fstat calls (#11150)
---
.../container/keyvalue/helpers/ChunkUtils.java | 35 +++--------
.../keyvalue/impl/FilePerBlockStrategy.java | 24 ++++----
.../container/keyvalue/helpers/TestChunkUtils.java | 12 +---
.../keyvalue/impl/TestFilePerBlockStrategy.java | 70 ++++++++++++++++++++++
4 files changed, 91 insertions(+), 50 deletions(-)
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
index 9cde9ba315e..fa55cbe793b 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
@@ -362,15 +362,14 @@ public static boolean validateChunkForOverwrite(File
chunkFile,
* Validates chunk data and returns a boolean value that indicates if the
* chunk data should be overwritten.
*
- * @param chunkFile - FileChannel of the chunkFile to write data into.
+ * @param fileLen - current length of the block file.
* @param info - chunk info.
* @return true if the chunkOffset is less than the chunkFile length,
* false otherwise.
*/
- public static boolean validateChunkForOverwrite(FileChannel chunkFile,
- ChunkInfo info) {
+ public static boolean validateChunkForOverwrite(long fileLen, ChunkInfo
info) {
- if (isOverWriteRequested(chunkFile, info)) {
+ if (isOverWriteRequested(fileLen, info)) {
if (!isOverWritePermitted(info)) {
LOG.warn("Duplicate write chunk request. Chunk overwrite " +
"without explicit request. {}", info);
@@ -407,24 +406,13 @@ public static boolean isOverWriteRequested(File
chunkFile, ChunkInfo
* Checks if a request to overwrite an existing range of a chunk has been
* received.
*
- * @param channel - FileChannel of the file to check
+ * @param fileLen - current length of the block file
* @param chunkInfo - Chunk information containing the offset
* @return true if the offset is less than the file length, indicating
* a request to overwrite an existing range; false otherwise
*/
- public static boolean isOverWriteRequested(FileChannel channel, ChunkInfo
- chunkInfo) {
- long fileLen;
- try {
- fileLen = channel.size();
- } catch (IOException e) {
- String msg = "IO error encountered while getting the file size";
- LOG.error(msg, e.getMessage());
- throw new UncheckedIOException("IO error encountered while " +
- "getting the file size for ", e);
- }
- long offset = chunkInfo.getOffset();
- return offset < fileLen;
+ public static boolean isOverWriteRequested(long fileLen, ChunkInfo
chunkInfo) {
+ return chunkInfo.getOffset() < fileLen;
}
/**
@@ -528,18 +516,9 @@ private static ContainerProtos.Result translate(Exception
cause) {
* Checks if the block file length is equal to the chunk offset.
*
*/
- public static void validateChunkSize(FileChannel fileChannel,
- ChunkInfo chunkInfo, String fileName)
+ public static void validateChunkSize(long fileLen, ChunkInfo chunkInfo,
String fileName)
throws StorageContainerException {
long offset = chunkInfo.getOffset();
- long fileLen;
- try {
- fileLen = fileChannel.size();
- } catch (IOException e) {
- throw new StorageContainerException("IO error encountered while " +
- "getting the file size for " + fileName + " at offset " + offset,
- CHUNK_FILE_INCONSISTENCY);
- }
if (fileLen != offset) {
throw new StorageContainerException("Chunk offset " + offset +
" does not match length " + fileLen + " of blockFile " + fileName,
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
index ecaf42a2059..c96721e9c81 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerBlockStrategy.java
@@ -17,7 +17,6 @@
package org.apache.hadoop.ozone.container.keyvalue.impl;
-import static
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CHUNK_FILE_INCONSISTENCY;
import static
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.GET_SHORT_CIRCUIT_FD_FAILED;
import static
org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.UNSUPPORTED_REQUEST;
import static
org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion.FILE_PER_BLOCK;
@@ -157,15 +156,24 @@ public void writeChunk(Container container, BlockID
blockID, ChunkInfo info,
HddsVolume volume = containerData.getVolume();
FileChannel channel = null;
- boolean overwrite;
try {
channel = files.getChannel(chunkFile, doSyncWrite);
- overwrite = validateChunkForOverwrite(channel, info);
} catch (IOException e) {
onFailure(volume);
throw e;
}
+ // Read the block file length once and reuse it for the overwrite check,
+ // the offset validation and the space accounting below.
+ long fileLengthBeforeWrite;
+ try {
+ fileLengthBeforeWrite = channel.size();
+ } catch (IOException e) {
+ throw new UncheckedIOException("Encountered an error while getting the
file size for " + chunkFile.getName(), e);
+ }
+
+ boolean overwrite = validateChunkForOverwrite(fileLengthBeforeWrite, info);
+
if (LOG.isDebugEnabled()) {
LOG.debug("Writing chunk {} (overwrite: {}) in stage {} to file {}",
info, overwrite, stage, chunkFile);
@@ -173,15 +181,7 @@ public void writeChunk(Container container, BlockID
blockID, ChunkInfo info,
// check whether offset matches block file length if its an overwrite
if (!overwrite) {
- ChunkUtils.validateChunkSize(channel, info, chunkFile.getName());
- }
-
- long fileLengthBeforeWrite;
- try {
- fileLengthBeforeWrite = channel.size();
- } catch (IOException e) {
- throw new StorageContainerException("Encountered an error while getting
the file size for "
- + chunkFile.getName(), CHUNK_FILE_INCONSISTENCY);
+ ChunkUtils.validateChunkSize(fileLengthBeforeWrite, info,
chunkFile.getName());
}
ChunkUtils.writeData(channel, chunkFile.getName(), data, offset,
chunkLength, volume);
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/helpers/TestChunkUtils.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/helpers/TestChunkUtils.java
index 86b2d5b6f79..4cf31c1ea79 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/helpers/TestChunkUtils.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/helpers/TestChunkUtils.java
@@ -32,7 +32,6 @@
import java.io.IOException;
import java.io.OutputStream;
import java.nio.ByteBuffer;
-import java.nio.channels.FileChannel;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
@@ -233,16 +232,9 @@ void validateChunkForOverwrite() throws IOException {
ChunkUtils.validateChunkForOverwrite(tempFile.toFile(),
new ChunkInfo("chunk", 5, 5)));
- try (FileChannel fileChannel =
- FileChannel.open(tempFile, StandardOpenOption.READ)) {
- assertTrue(
- ChunkUtils.validateChunkForOverwrite(fileChannel,
- new ChunkInfo("chunk", 3, 5)));
+ assertTrue(ChunkUtils.validateChunkForOverwrite(4L, new ChunkInfo("chunk",
3, 5)));
- assertFalse(
- ChunkUtils.validateChunkForOverwrite(fileChannel,
- new ChunkInfo("chunk", 5, 5)));
- }
+ assertFalse(ChunkUtils.validateChunkForOverwrite(4L, new
ChunkInfo("chunk", 5, 5)));
}
@Test
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java
index b79a2984485..32be74ca7b1 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java
@@ -21,17 +21,32 @@
import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY;
import static org.apache.hadoop.ozone.container.ContainerTestHelper.getChunk;
+import static
org.apache.hadoop.ozone.container.ContainerTestHelper.getWriteChunkRequest;
import static
org.apache.hadoop.ozone.container.ContainerTestHelper.setDataChecksum;
import static
org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch;
import static
org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE;
import static
org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet;
+import static
org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion.FILE_PER_BLOCK;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.AdditionalAnswers.delegatesTo;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
import java.io.File;
import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.io.UncheckedIOException;
import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.MessageDigest;
@@ -45,6 +60,7 @@
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
import
org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
+import org.apache.hadoop.hdds.scm.pipeline.MockPipeline;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.common.ChunkBuffer;
import org.apache.hadoop.ozone.common.ChunkBufferToByteString;
@@ -68,6 +84,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
+import org.mockito.MockedConstruction;
/**
* Test for FilePerBlockStrategy.
@@ -77,6 +94,59 @@ public class TestFilePerBlockStrategy extends
CommonChunkManagerTestCases {
@TempDir
private File tempDir;
+ @Test
+ public void testWriteChunkReadsFileSizeOnce() throws IOException {
+ File chunkFile = FILE_PER_BLOCK.getChunkFile(getKeyValueContainerData(),
getBlockID(), null);
+ byte[] expectedData = new byte[getData().remaining()];
+ getData().duplicate().get(expectedData);
+ KeyValueHandler handler = createKeyValueHandler(newContainerSet());
+ try (RandomAccessFile actualFile = new RandomAccessFile(chunkFile, "rw")) {
+ FileChannel channel = mock(FileChannel.class,
delegatesTo(actualFile.getChannel()));
+ try (MockedConstruction<RandomAccessFile> files =
mockConstruction(RandomAccessFile.class,
+ (file, context) -> when(file.getChannel()).thenReturn(channel))) {
+ handler.getChunkManager().writeChunk(
+ getKeyValueContainer(), getBlockID(), getChunkInfo(), getData(),
WRITE_STAGE);
+
+ verify(channel).size();
+ assertThat(files.constructed()).hasSize(1);
+
assertThat(Files.readAllBytes(chunkFile.toPath())).isEqualTo(expectedData);
+ checkWriteIOStats(expectedData.length, 1);
+
assertThat(getKeyValueContainerData().getBytesUsed()).isEqualTo(expectedData.length);
+ }
+ } finally {
+ handler.stop();
+ }
+ }
+
+ @Test
+ public void testFileSizeFailure() throws IOException {
+ KeyValueHandler handler = createKeyValueHandler(newContainerSet());
+ ContainerProtos.ContainerCommandRequestProto request =
+ getWriteChunkRequest(MockPipeline.createSingleNodePipeline(),
getBlockID(), 20);
+ FileChannel channel = mock(FileChannel.class);
+ IOException failure = new IOException("Failed to read file size");
+ when(channel.size()).thenThrow(failure);
+
+ try (MockedConstruction<RandomAccessFile> files =
+ mockConstruction(RandomAccessFile.class, (file, context) ->
when(file.getChannel()).thenReturn(channel))) {
+ assertThatThrownBy(() -> handler.getChunkManager().writeChunk(
+ getKeyValueContainer(), getBlockID(), getChunkInfo(), getData(),
WRITE_STAGE))
+ .isInstanceOf(UncheckedIOException.class)
+ .hasCause(failure);
+
+ ContainerProtos.ContainerCommandResponseProto response =
+ handler.handle(request, getKeyValueContainer(), WRITE_STAGE);
+
assertThat(response.getResult()).isEqualTo(ContainerProtos.Result.CONTAINER_INTERNAL_ERROR);
+ assertThat(files.constructed()).hasSize(1);
+ verify(channel, times(2)).size();
+ verifyNoMoreInteractions(channel);
+ checkWriteIOStats(0, 0);
+ assertThat(getKeyValueContainerData().getBytesUsed()).isZero();
+ } finally {
+ handler.stop();
+ }
+ }
+
@Test
public void testDeletePartialChunkWithOffsetUnsupportedRequest() {
// GIVEN
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
