From 41ad61789f3d2ca92959a4fc8d54575d441304eb Mon Sep 17 00:00:00 2001 From: andyhuangdev Date: Mon, 12 Oct 2026 18:06:05 +0800 Subject: [PATCH 1/2] HDDS-16874. Fix overwrite detection in FilePerChunkStrategy Use the file-relative write offset to detect existing chunk data in FILE_PER_CHUNK containers. This prevents retries of non-first chunks from double-counting bytesUsed or failing COMMIT_DATA when the temporary file has already been moved. Add regression tests for COMBINED retries and repeated COMMIT_DATA at zero and nonzero block offsets. Tests: 44 tests passed across TestFilePerChunkStrategy, TestFilePerBlockStrategy, and TestChunkUtils. Checkstyle passed. --- .../keyvalue/helpers/ChunkUtils.java | 8 +++- .../keyvalue/impl/FilePerChunkStrategy.java | 4 +- .../impl/TestFilePerChunkStrategy.java | 40 +++++++++++++++++++ 3 files changed, 49 insertions(+), 3 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 dcee2507d660..13d85493531b 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 b210f9d730e7..8d7920ebef99 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/keyvalue/impl/TestFilePerChunkStrategy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerChunkStrategy.java index 6c4694e86b37..59a8467615cb 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(); From 66be8dc9ca231014f93894bb03c58afad35c7924 Mon Sep 17 00:00:00 2001 From: andyhuangdev Date: Tue, 13 Oct 2026 10:12:30 +0800 Subject: [PATCH 2/2] HDDS-16874. Use unique chunk names in TestContainerPersistence Use the existing getChunk helper so distinct chunks have unique file names. This prevents FILE_PER_CHUNK writes from being treated as overwrites and preserves the expected bytesUsed assertion. Tests: Reproduced the four failing parameter combinations before the fix; all eight passed afterward. Related suites completed with 220 tests, zero failures or errors, and eight skipped. Checkstyle and git diff --check passed. --- .../ozone/container/common/impl/TestContainerPersistence.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 0498d849961e..c107e0e16e1e 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,