Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -332,44 +332,18 @@ private static List<ByteBuf> readDataNettyChunkedNioFile(
return buffers;
}

/**
* Validates chunk data and returns a file object to Chunk File that we are
* expected to write data to.
*
* @param chunkFile - chunkFile to write data into.
* @param info - chunk info.
* @return true if the chunkFile exists and chunkOffset &lt; chunkFile length,
* false otherwise.
*/
public static boolean validateChunkForOverwrite(File chunkFile,
ChunkInfo info) {

if (isOverWriteRequested(chunkFile, info)) {
if (!isOverWritePermitted(info)) {
LOG.warn("Duplicate write chunk request. Chunk overwrite " +
"without explicit request. {}", info);
}
return true;
}

// TODO: when overwriting a chunk, we should ensure that the new chunk
// size is same as the old chunk size

return false;
}

/**
* Validates chunk data and returns a boolean value that indicates if the
* chunk data should be overwritten.
*
* @param fileLen - current length of the block file.
* @param fileLen - current length of the chunk or block file.
* @param info - chunk info.
* @return true if the chunkOffset is less than the chunkFile length,
* false otherwise.
*/
public static boolean validateChunkForOverwrite(long fileLen, ChunkInfo info) {

if (isOverWriteRequested(fileLen, info)) {
if (info.getOffset() < fileLen) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but I noticed that comparing file length with the block-relative offset may miss overwrites in chunks after the first. Filed HDDS-16874 as a follow-up.

if (!isOverWritePermitted(info)) {
LOG.warn("Duplicate write chunk request. Chunk overwrite " +
"without explicit request. {}", info);
Expand All @@ -383,38 +357,6 @@ public static boolean validateChunkForOverwrite(long fileLen, ChunkInfo info) {
return false;
}

/**
* Checks if we are getting a request to overwrite an existing range of
* chunk.
*
* @param chunkFile - File
* @param chunkInfo - Buffer to write
* @return bool
*/
public static boolean isOverWriteRequested(File chunkFile, ChunkInfo
chunkInfo) {

if (!chunkFile.exists()) {
return false;
}

long offset = chunkInfo.getOffset();
return offset < chunkFile.length();
}

/**
* Checks if a request to overwrite an existing range of a chunk has been
* received.
*
* @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(long fileLen, ChunkInfo chunkInfo) {
return chunkInfo.getOffset() < fileLen;
}

/**
* Overwrite is permitted if an only if the user explicitly asks for it. We
* permit this iff the key/value pair contains a flag called
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ public void writeChunk(Container container, BlockID blockID, ChunkInfo info,

File chunkFile = getChunkFile(kvContainer, blockID, info);

boolean isOverwrite = ChunkUtils.validateChunkForOverwrite(
chunkFile, info);
boolean isOverwrite = ChunkUtils.validateChunkForOverwrite(chunkFile.length(), info);
File tmpChunkFile = getTmpChunkFile(chunkFile, dispatcherContext);
if (LOG.isDebugEnabled()) {
LOG.debug(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,19 +219,7 @@ void serialRead() throws IOException {
}

@Test
void validateChunkForOverwrite() throws IOException {

Path tempFile = tempDir.toPath().resolve("overwrite");
FileUtils.write(tempFile.toFile(), "test", UTF_8);

assertTrue(
ChunkUtils.validateChunkForOverwrite(tempFile.toFile(),
new ChunkInfo("chunk", 3, 5)));

assertFalse(
ChunkUtils.validateChunkForOverwrite(tempFile.toFile(),
new ChunkInfo("chunk", 5, 5)));

void validateChunkForOverwrite() {
assertTrue(ChunkUtils.validateChunkForOverwrite(4L, new ChunkInfo("chunk", 3, 5)));

assertFalse(ChunkUtils.validateChunkForOverwrite(4L, new ChunkInfo("chunk", 5, 5)));
Expand Down
Loading