Skip to content

HDDS-16874. Fix overwrite detection in FilePerChunkStrategy #11490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jojochuang merged 2 commits into from
Oct 13, 2026
Merged

HDDS-16874. Fix overwrite detection in FilePerChunkStrategy #11490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41ad617
HDDS-16874. Fix overwrite detection in FilePerChunkStrategy
andyhuangdev Oct 12, 2026
66be8dc
HDDS-16874. Use unique chunk names in TestContainerPersistence
andyhuangdev Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion ...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,14 @@ private static List<ByteBuf> 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);
Expand Down
4 changes: 2 additions & 2 deletions ...e/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/FilePerChunkStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions ...src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,7 @@ public void testPutBlockWithLotsOfChunks(ContainerTestVersionInfo versionInfo)
BlockID blockID = ContainerTestHelper.getTestBlockID(testContainerID);
List<ChunkInfo> 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,
Expand Down
40 changes: 40 additions & 0 deletions ...c/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerChunkStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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();
Expand Down