Siyao Meng created HDDS-17677:
---------------------------------
Summary: FILE_PER_BLOCK container bytes used is saved too low
after a datanode restart between data write and apply
Key: HDDS-17677
URL: https://issues.apache.org/jira/browse/HDDS-17677
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
CR-14-load-open-file-per-block-bytes-used-from-block-files.patch,
TestBugCR14BytesUsedAfterRestartReplay.java
h3. Mechanism
With the FILE_PER_BLOCK layout, {{FilePerBlockStrategy.writeChunk}} adds the
growth of the block file to the container counter in the WRITE_DATA stage,
before the log entry is applied, and ignores the COMMIT_DATA stage.
{{BlockManagerImpl.persistPutBlock}} saves the counter as {{#BYTESUSED}} on
every PutBlock, and {{KeyValueContainerUtil.populateBlockStatistics}} loads
that value at startup.
After a restart, Ratis replays committed entries through {{applyTransaction}}
only. A replayed WriteChunk comes without data and in the COMMIT_DATA stage, so
the bytes it wrote before the restart are not counted again. The container was
loaded with the value saved by the last PutBlock applied before the restart,
which does not include data written after that PutBlock, and the replayed
PutBlock saves that low value. Nothing recomputes it later, so it stays low
while the block files and the committed blocks hold more.
FILE_PER_CHUNK is not affected: it counts at COMMIT_DATA, which commits the tmp
chunk file that survives the restart.
h3. Trigger
# A container with the FILE_PER_BLOCK layout (the default) is open on a Ratis
pipeline, or already marked for close (CLOSING) with log entries still to apply.
# The data of a WriteChunk is written on a datanode (leader or follower) after
the last PutBlock applied there on that container.
# The datanode process stops before a PutBlock on that container is applied
after that write; the PutBlock is applied by log replay after the restart.
No fault beyond a restart is needed. A follower that lags in applying entries
has a larger window.
h3. Impact
The bytes used of that replica is permanently below its block files and its
committed blocks, by the data written but not applied at the restart, and the
error adds up over restarts while the container is open. The low value is what
the replica reports to SCM as {{used}} and what
{{HddsDispatcher.isContainerFull}} uses to request the close of a full
container (argued from the code, not run). SCM takes the smallest reported
value while the container is open and the largest after that, so its own view
stays low only while the container is open, or after close if every replica is
low. Accounting only, no data is lost.
Separately, {{#BYTESUSED}} can be above the sum of committed block lengths
while data of entries not yet applied is in the block files. That matches what
the counter tracks for FILE_PER_BLOCK and is not part of this report.
h3. Reproduction
Fails on the unfixed code, deterministic, unmodified source.
[^TestBugCR14BytesUsedAfterRestartReplay.java]
({{hadoop-hdds/container-service}}) sends WriteChunk and PutBlock through
{{KeyValueHandler.handle}} with the contexts {{ContainerStateMachine}} builds
for writeStateMachineData and applyTransaction: index 1 and 2 for block 1 are
applied, the data of index 3 (block 2) is written, then the DB cache is closed
and the container is reloaded as {{ContainerReader}} does, and index 3 and 4
are replayed. For FILE_PER_BLOCK it fails with {{expected: <2048> but was:
<1024>}} (block files 2048 bytes, {{#BYTESUSED}} 1024). For FILE_PER_CHUNK it
passes. The restart is simulated in one JVM, no mini cluster was used.
h3. Patch
[^CR-14-load-open-file-per-block-bytes-used-from-block-files.patch], against
ec5ed5f7ea0e4b406aee53aae4906616819cba2b. It also applies to master at
7294fc2da18.
When an OPEN or CLOSING FILE_PER_BLOCK container is loaded, its bytes used
becomes the larger of the saved {{#BYTESUSED}} and the length of the files in
its chunks directory. These are the states in which {{KeyValueHandler}} still
applies WriteChunk and PutBlock, and loading happens before log replay, so the
replayed PutBlock saves the right value. Counting during writes is unchanged,
so space checks behave as today; only the value loaded for such a container can
go up, at the cost of listing its chunks directory at startup. If the directory
cannot be listed, the saved value is kept and a warning is logged. Containers
in other states (CLOSED, QUASI_CLOSED and the rest) and FILE_PER_CHUNK
containers load as before.
Counting FILE_PER_BLOCK bytes at apply instead would delay space accounting for
data already on disk and cannot compute the growth of an overwrite at that
point. A separate counter of committed lengths would need a read of the stored
block on every PutBlock.
Covered by {{testBytesUsedAfterRestartBeforeApply}} (an OPEN and a CLOSING
case) and {{testBytesUsedOfClosedContainerIsLoadedFromDb}} in the existing
{{TestFilePerBlockStrategy}}. Without the change both cases of the first fail
with {{expected: <40> but was: <20>}}; the second checks that a CLOSED
container keeps the saved value. With the change the container service tests
pass, apart from 16 tests that fail the same way on the unfixed code.
Found by code review of the datanode container state machine apply and block
commit paths, as part of the TLA+ verification effort under HDDS-15926, on
commit ec5ed5f7ea0e4b406aee53aae4906616819cba2b. Checked against HDDS issues
and apache/ozone pull requests for duplicates before filing. The attached patch
is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
