[
https://issues.apache.org/jira/browse/HDDS-17675?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17675:
------------------------------
Attachment: TestBugMC2StaleWriteFutureAfterTruncate.java
> 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
> Affects Versions: 2.0.0, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1, 2.2.2
> Reporter: Siyao Meng
> Priority: Major
> 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]
