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 dcee2507d66..13d85493531 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 @@ -342,8 +342,14 @@ private static List readDataNettyChunkedNioFile( * false otherwise. */ public static boolean validateChunkForOverwrite(long fileLen, ChunkInfo info) { + return validateChunkForOverwrite(fileLen, info, info.getOffset()); + } - if (info.getOffset() < fileLen) { + /** + * Checks for an overwrite using the write offset within the file. + */ + public static boolean validateChunkForOverwrite(long fileLen, ChunkInfo info, long fileOffset) { + if (fileOffset < fileLen) { if (!isOverWritePermitted(info)) { LOG.warn("Duplicate write chunk request. Chunk overwrite " + "without explicit request. {}", info); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerChunkStrategy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerChunkStrategy.java index b210f9d730e..8d7920ebef9 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerChunkStrategy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerChunkStrategy.java @@ -118,7 +118,8 @@ public void writeChunk(Container container, BlockID blockID, ChunkInfo info, File chunkFile = getChunkFile(kvContainer, blockID, info); - boolean isOverwrite = ChunkUtils.validateChunkForOverwrite(chunkFile.length(), info); + long offset = 0; // ignore offset in chunk info + boolean isOverwrite = ChunkUtils.validateChunkForOverwrite(chunkFile.length(), info, offset); File tmpChunkFile = getTmpChunkFile(chunkFile, dispatcherContext); if (LOG.isDebugEnabled()) { LOG.debug( @@ -127,7 +128,6 @@ public void writeChunk(Container container, BlockID blockID, ChunkInfo info, } long len = info.getLen(); - long offset = 0; // ignore offset in chunk info switch (stage) { case WRITE_DATA: if (isOverwrite) { diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java index 0498d849961..c107e0e16e1 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java @@ -980,8 +980,7 @@ public void testPutBlockWithLotsOfChunks(ContainerTestVersionInfo versionInfo) BlockID blockID = ContainerTestHelper.getTestBlockID(testContainerID); List chunkList = new LinkedList<>(); for (int x = 0; x < chunkCount; x++) { - ChunkInfo info = new ChunkInfo(String.format("%d.data", - blockID.getLocalID()), x * datalen, datalen); + ChunkInfo info = getChunk(blockID.getLocalID(), x, x * datalen, datalen); ChunkBuffer data = getData(datalen); setDataChecksum(info, data); chunkManager.writeChunk(container, blockID, info, data, diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerChunkStrategy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerChunkStrategy.java index 6c4694e86b3..59a8467615c 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerChunkStrategy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerChunkStrategy.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.container.keyvalue.impl; +import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.COMBINED_STAGE; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.COMMIT_STAGE; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -34,6 +35,8 @@ import org.apache.hadoop.ozone.container.keyvalue.helpers.ChunkUtils; import org.apache.hadoop.ozone.container.keyvalue.interfaces.ChunkManager; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Test for FilePerChunkStrategy. @@ -45,6 +48,43 @@ protected ContainerLayoutTestInfo getStrategy() { return ContainerLayoutTestInfo.FILE_PER_CHUNK; } + @ParameterizedTest + @ValueSource(ints = {0, 1, 2}) + public void testCombinedRetryDoesNotIncreaseBytesUsed(int chunkIndex) throws Exception { + ChunkManager chunkManager = createTestSubject(); + long len = getChunkInfo().getLen(); + ChunkInfo info = new ChunkInfo(getBlockID().getLocalID() + ".data." + chunkIndex, chunkIndex * len, len); + + chunkManager.writeChunk(getKeyValueContainer(), getBlockID(), info, getData(), COMBINED_STAGE); + assertEquals(len, getKeyValueContainerData().getBytesUsed()); + rewindBufferToDataStart(); + chunkManager.writeChunk(getKeyValueContainer(), getBlockID(), info, getData(), COMBINED_STAGE); + + checkChunkFileCount(1); + assertEquals(len, getKeyValueContainerData().getBytesUsed()); + } + + @ParameterizedTest + @ValueSource(ints = {0, 1, 2}) + public void testRepeatedCommitWithMissingTemporaryFile(int chunkIndex) throws Exception { + ChunkManager chunkManager = createTestSubject(); + long len = getChunkInfo().getLen(); + ChunkInfo info = new ChunkInfo(getBlockID().getLocalID() + ".data." + chunkIndex, chunkIndex * len, len); + + chunkManager.writeChunk(getKeyValueContainer(), getBlockID(), info, getData(), WRITE_STAGE); + chunkManager.writeChunk(getKeyValueContainer(), getBlockID(), info, getData(), COMMIT_STAGE); + File chunkFile = ContainerLayoutVersion.FILE_PER_CHUNK.getChunkFile( + getKeyValueContainerData(), getBlockID(), info.getChunkName()); + assertTrue(chunkFile.exists()); + checkChunkFileCount(1); + assertEquals(len, getKeyValueContainerData().getBytesUsed()); + + chunkManager.writeChunk(getKeyValueContainer(), getBlockID(), info, getData(), COMMIT_STAGE); + + checkChunkFileCount(1); + assertEquals(len, getKeyValueContainerData().getBytesUsed()); + } + @Test public void testWriteChunkStageWriteAndCommit() throws Exception { ChunkManager chunkManager = createTestSubject();