Repository navigation
HDDS-16874. Fix overwrite detection in FilePerChunkStrategy - #11490
Merged
Merged
Conversation
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.
jojochuang
approved these changes
Oct 12, 2026
jojochuang
left a comment
jojochuang
left a comment
Contributor
There was a problem hiding this comment.
+1 FilePerChunk is deprecated, but still a good fix.
Contributor
|
The test TestContainerPersistence failure looks related. |
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.
Contributor
Author
The test used the same name for two distinct chunks, so the corrected overwrite detection exposed the issue. I've updated it to use the existing getChunk helper for unique chunk names and verified that the previously failing test passes in my fork. |
Contributor
|
Merged. Thanks @andyhuangdev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This patch fixes overwrite detection for existing FILE_PER_CHUNK containers. Chunk offsets are relative to the block, but each chunk is stored in a separate file starting at offset zero. Comparing the block offset with the chunk file length incorrectly treats retries of non-first chunks as new writes.
This causes COMBINED retries to double-count container bytesUsed. Repeated COMMIT_DATA also fails when the temporary file has already been moved to the final chunk file.
The patch extends the existing overwrite helper to accept a file-relative offset and uses offset zero for FILE_PER_CHUNK. FILE_PER_BLOCK retains its existing behavior. Regression tests cover both retry paths at zero and nonzero block offsets.
What is the link to the Apache JIRA
HDDS-16874
How was this patch tested?
The regression tests reproduced both issues before the fix: nonzero-offset COMBINED retries increased bytesUsed from 20 to 40, and repeated COMMIT_DATA failed with NoSuchFileException. Zero-offset cases passed.
After the fix, all 44 tests passed across TestFilePerChunkStrategy, TestFilePerBlockStrategy, and TestChunkUtils. The project checkstyle script and git diff --check also passed.
Datanode restart and pipeline closure were not tested end to end.