Siyao Meng created HDDS-17675:
---------------------------------
Summary: Datanode acknowledges and commits a WriteChunk without
writing its data when a truncated entry at the same log index still has a
pending write
Key: HDDS-17675
URL: https://issues.apache.org/jira/browse/HDDS-17675
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-2-bind-pending-write-future-to-entry-term.patch,
TestBugMC2StaleWriteFutureAfterTruncate.java
h3. Mechanism
{{ContainerStateMachine.writeStateMachineData}} keeps the pending data write of
each WriteChunk in {{writeChunkFutureMap}}, keyed by log index only. When an
entry for that index is already present, it returns the existing future without
checking the term or the request (added by HDDS-12469 as a precaution for a
retried write of the same entry). A map entry is removed only when its chunk
write task finishes; {{truncate}} and {{notifyNotLeader}} only touch
{{stateMachineDataCache}}.
In Ratis 3.3.1, {{SegmentedRaftLog.appendImpl}} calls {{StateMachine.truncate}}
and then {{StateMachine.write}} for each new entry while it builds the log
tasks under the log write lock. Nothing waits for data writes that this
datanode started for its own entries while it was leader, because those appends
are not part of the follower append chain
({{RaftServerImpl.appendEntriesAsync}}). So if such an entry is truncated while
its chunk write is still queued, and the new leader puts a different WriteChunk
at the same index, {{write}} returns the future of the truncated entry:
* the new chunk is never written;
* the old task's completion completes the new entry's write and {{flush}}, so
the datanode acknowledges the entry;
* the COMMIT_DATA apply of the WriteChunk succeeds, because
{{FilePerBlockStrategy.writeChunk}} does nothing in that stage;
* a following PutBlock succeeds and the container BCSID moves past the missing
chunk.
h3. Trigger
# Datanode A is leader of a Ratis pipeline. Its data writes for new WriteChunk
entries are queued behind a slow volume (the condition HDDS-12469 describes).
# A loses leadership before those entries commit (for example a network
partition), and another datanode becomes leader in a higher term without them.
# A receives the new leader's entries. They truncate A's uncommitted entries,
and a different WriteChunk lands on an index whose old data write is still
pending. The old write must still be pending when the new leader's entries
arrive (at least the time of an election) but younger than
{{hdds.container.ratis.statemachine.write.wait.interval}} (default 10m), after
which the next write marks the state machine unhealthy.
# In the case that was run, the swallowed chunk is the last chunk of a block
before its PutBlock(eof), and the block file already exists from an earlier
chunk, so the file check of PutBlock(eof) passes. Other placements of the
swallowed chunk were not run.
Needs two faults at once (leadership loss and a slow volume on the old leader),
so it is rare. No client action can cause it.
The same reuse can happen for entries the datanode received as a follower, but
only in the short window between the completion of the old write future and its
removal from the map, because Ratis 3.3.1 chains follower appends (append
entries compose, enabled by default). Ozone 2.0.0 ships Ratis 3.1.2, which does
not chain them, so there this window is likely wider. That variant is argued
from the code, not run.
h3. Impact
On the affected replica the container stays OPEN and healthy, its BCSID and the
block metadata (GetBlock length) include the chunk, but the block file is short
and ReadChunk of the committed chunk fails ({{CONTAINER_INTERNAL_ERROR}},
"Unexpected read size"). Nothing on the write path repairs it. A reader served
by this replica gets an error and has to use another replica, and the container
data scanner skips OPEN containers, so the replica counts as good until the
container is closed and scanned. The replica's acknowledgement also counts
towards the commit of a chunk it does not hold; with one other replica lagging,
the write could be committed with a single good copy (argued from the Ratis
code, not run).
h3. Reproduction
[^TestBugMC2StaleWriteFutureAfterTruncate.java]
({{hadoop-hdds/container-service}}) drives the real {{ContainerStateMachine}},
{{HddsDispatcher}}, {{KeyValueHandler}}, {{FilePerBlockStrategy}}, an on disk
container and the real Ratis {{SegmentedRaftLog}}, which issues the write,
truncate and flush calls itself. {{RaftServerImpl}} and the other peers are not
run; the test calls the log in the order {{RaftServerImpl}} does. The slow
volume is one of the two chunk executors kept busy. Deterministic, unmodified
source. On an unpatched build {{tailChunkOfNewLeaderIsCommittedWithoutItsData}}
passes, which means the defect is present:
{noformat}
dataWrites(block@offset)=[100@0, 10@0, 10@4096, 2@0, 11@0]
applyResults=[SUCCESS, SUCCESS, SUCCESS] containerState=OPEN containerBCSID=5
lastIndex=5 block2Length=8192 block2FileLength=4096 readChunk(block 2, chunk
1)=CONTAINER_INTERNAL_ERROR readMatches=false
{noformat}
The data write of block 2 at offset 4096 never runs; the only write at that
index is the truncated entry's chunk of block 11. {{controlWithoutSlowVolume}}
runs the same sequence with the old writes finished before the truncation and
reads the correct data.
h3. Patch
[^MC-2-bind-pending-write-future-to-entry-term.patch], against
ec5ed5f7ea0e4b406aee53aae4906616819cba2b. It also applies to master at
7294fc2da18.
{{WriteFutures}} records the term of its entry, and {{writeStateMachineData}}
reuses a pending future only when the term matches, so a retried write of the
same entry still gets the existing future (the HDDS-12469 intent), while a
different entry at a truncated index starts its own write and replaces the map
entry. The finishing task of the truncated entry then removes the map entry
only if it is still its own, so that {{flush}} and the write timeout check keep
tracking the new entry. {{truncate}} does not need to drop map entries:
leftovers of truncated entries are no longer reused and leave the map when
their task finishes, as today.
Covered by {{testWriteAfterTruncateDoesNotReuseFutureOfTruncatedEntry}} in the
existing {{ContainerStateMachineTests}} (run as leader and follower). Without
the change it fails because the new WriteChunk is never dispatched; with only
the term check it fails because {{flush}} no longer waits for the new write.
With the patch {{TestContainerStateMachineLeader}},
{{TestContainerStateMachineFollower}} and the mini cluster suites
{{TestContainerStateMachine}}, {{TestContainerStateMachineFailures}},
{{TestContainerStateMachineFlushDelay}} and {{TestCSMMetrics}} pass, and the
reproduction fails at its first assertion because block 2 is written in full
and reads back correctly.
Found by TLA+ model checking and code review of the datanode container state
machine apply and block commit paths 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]
