[
https://issues.apache.org/jira/browse/HDDS-16874?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chi-Hsuan Huang updated HDDS-16874:
-----------------------------------
Description:
`FilePerChunkStrategy.writeChunk` decides whether a write is an overwrite with
`info.getOffset() < chunkFile.length()`. Since HDDS-2717 the chunk offset is
relative to the block, but in the FILE_PER_CHUNK layout each chunk has its own
file. For any chunk after the first, the offset is at least one chunk length,
so a rewrite of an existing chunk is not detected.
I found two effects with unit tests on `TestFilePerChunkStrategy` (second
chunk, offset = len):
1. A COMBINED retry counts the chunk twice in container `bytesUsed` (expected
20, actual 40).
2. A repeated COMMIT_DATA, when the chunk file exists and the tmp file is gone,
fails with `NoSuchFileException`.
The first chunk (offset 0) behaves correctly in both cases.
On datanode restart, `ContainerStateMachine.applyTransaction` replays
WriteChunk entries after the last snapshot as COMMIT_DATA, and `HddsDispatcher`
does not skip already applied entries. So effect 2 could fail the apply and
trigger a pipeline close. I have not reproduced this end to end.
**Expected behavior**
A rewrite of any existing chunk in a FILE_PER_CHUNK container should be treated
as an overwrite.
**Notes**
FILE_PER_CHUNK is deprecated for new containers (HDDS-11753), so this only
affects existing containers in that layout. Found while reviewing HDDS-16844,
which keeps the current behavior.
was:
{{FilePerChunkStrategy.writeChunk}} decides whether a write is an overwrite
with {{info.getOffset\(\) < chunkFile.length\(\)}}. Since HDDS\-2717 the chunk
offset is relative to the block, but in the FILE\_PER\_CHUNK layout each chunk
has its own file. For any chunk after the first, the offset is at least one
chunk length, so a rewrite of an existing chunk is not detected.
I found two effects with unit tests on {{TestFilePerChunkStrategy}} \(second
chunk, offset = len\):h1. A COMBINED retry counts the chunk twice in container
{{bytesUsed}} \(expected 20, actual 40\).
h1. A repeated COMMIT\_DATA, when the chunk file exists and the tmp file is
gone, fails with {{NoSuchFileException}}.
The first chunk \(offset 0\) behaves correctly in both cases.
On datanode restart, {{ContainerStateMachine.applyTransaction}} replays
WriteChunk entries after the last snapshot as COMMIT\_DATA, and
{{HddsDispatcher}} does not skip already applied entries. So effect 2 could
fail the apply and trigger a pipeline close. I have not reproduced this end to
end.
_Expected behavior_
A rewrite of any existing chunk in a FILE\_PER\_CHUNK container should be
treated as an overwrite.
_Notes_
FILE\_PER\_CHUNK is deprecated for new containers \(HDDS\-11753\), so this only
affects existing containers in that layout. Found while reviewing HDDS\-16844,
which keeps the current behavior.
> FilePerChunk overwrite check misses chunks after the first in a block
> ---------------------------------------------------------------------
>
> Key: HDDS-16874
> URL: https://issues.apache.org/jira/browse/HDDS-16874
> Project: Apache Ozone
> Issue Type: Bug
> Components: Ozone Datanode
> Reporter: Chi-Hsuan Huang
> Priority: Minor
>
> `FilePerChunkStrategy.writeChunk` decides whether a write is an overwrite
> with `info.getOffset() < chunkFile.length()`. Since HDDS-2717 the chunk
> offset is relative to the block, but in the FILE_PER_CHUNK layout each chunk
> has its own file. For any chunk after the first, the offset is at least one
> chunk length, so a rewrite of an existing chunk is not detected.
> I found two effects with unit tests on `TestFilePerChunkStrategy` (second
> chunk, offset = len):
> 1. A COMBINED retry counts the chunk twice in container `bytesUsed` (expected
> 20, actual 40).
> 2. A repeated COMMIT_DATA, when the chunk file exists and the tmp file is
> gone, fails with `NoSuchFileException`.
> The first chunk (offset 0) behaves correctly in both cases.
> On datanode restart, `ContainerStateMachine.applyTransaction` replays
> WriteChunk entries after the last snapshot as COMMIT_DATA, and
> `HddsDispatcher` does not skip already applied entries. So effect 2 could
> fail the apply and trigger a pipeline close. I have not reproduced this end
> to end.
> **Expected behavior**
> A rewrite of any existing chunk in a FILE_PER_CHUNK container should be
> treated as an overwrite.
> **Notes**
> FILE_PER_CHUNK is deprecated for new containers (HDDS-11753), so this only
> affects existing containers in that layout. Found while reviewing HDDS-16844,
> which keeps the current behavior.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
