Siyao Meng created HDDS-16824:
---------------------------------
Summary: Lease recovery takes the block length from the first
replica it can ask, so a lagging replica truncates hsynced data
Key: HDDS-16824
URL: https://issues.apache.org/jira/browse/HDDS-16824
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-5-lease-recovery-use-largest-replica-block-length.patch,
TestBugMC5RecoveredLengthFromLaggingReplica.java
h3. Mechanism
When {{finalizeBlock}} in {{BasicRootedOzoneClientAdapterImpl}} or
{{BasicOzoneClientAdapterImpl}} cannot finalize a block through its Ratis
pipeline (the pipeline is no longer open, or FinalizeBlock failed), it falls
back to {{ContainerProtocolCalls.getCommittedBlockLength}} on a standalone
pipeline made of the datanodes of the block. {{XceiverClientGrpc}} asks the
datanodes one after another and returns the first successful answer, and
{{LeaseRecoveryClientDNHandler}} commits the file with that length.
Nothing on this path tells replicas apart. hsync commits the block to OM with
the block ID as allocated, whose block commit sequence ID is 0, so
{{BlockUtils.verifyBCSId}} on the datanode accepts every replica, and a replica
that is still open answers {{GetCommittedBlockLength}} like a closed one. A
replica that missed the last PutBlock of the block reports the shorter length,
and the file is closed with it.
With the default {{ozone.network.topology.aware.read=true}} the datanodes are
asked in the order in which SCM returned them, otherwise in random order.
h3. Trigger
# A client writes a RATIS/THREE file and calls hsync.
# One datanode of the pipeline stops. hsync keeps succeeding: after
{{hdds.ratis.raft.client.rpc.watch.request.timeout}} (default 30s) the client
stops waiting for all replicas and accepts a commit by the majority.
# The writer dies while the datanode is down, before an hsync in a next block
has reached OM. The container is closed and the pipeline removed before the
datanode returns, so it cannot catch up and its replica keeps the shorter block.
# {{recoverLease}} is called after the datanode is back and before
ReplicationManager has replaced its replica, and that datanode is the first one
asked.
Steps 1, 2 and 4 are run as written. In step 3 the reproduction closes the
container and removes the pipeline with SCM calls. That SCM does both on its
own for a datanode that stays down longer than
{{ozone.scm.stale.node.interval}} (default 5m), and that ReplicationManager
needs minutes to replace the replica ({{hdds.scm.replication.thread.interval}}
is 300s), is from reading the code, not run. Step 3 is one way to get there.
What the fallback needs is any replica that is behind and reachable when it
runs, for example a follower that is still replaying its log after a restart
(argued, not run).
h3. Impact
The recovered file is shorter than what hsync acknowledged, although two of
three replicas hold the data. In the reproduction the second hsync of 100 bytes
returned successfully and the file is closed with 100 bytes. For a write ahead
log this is silent loss of acknowledged entries. The length is final once the
file is closed, and the bytes beyond it on the two healthy replicas are no
longer reachable through the file.
h3. Reproduction
PASS on unmodified source, nothing injected, recovery not forced.
[^TestBugMC5RecoveredLengthFromLaggingReplica.java]
({{hadoop-ozone/integration-test}}) starts a mini cluster with three datanodes.
The writer hsyncs 100 bytes, the datanode whose replica SCM lists first is
stopped, and the writer hsyncs 100 more bytes. The container is closed and its
pipeline removed, the datanode is started again and still has 100 bytes of the
block, the other two have 200. {{recoverLease}} returns true and the file is
closed with 100 bytes. The only non default settings are those that enable
hsync, a lease soft limit of zero and
{{hdds.ratis.raft.client.rpc.watch.request.timeout}} of 3s instead of 30s. A
passing test means the defect is present.
h3. Suggested fix
The attached patch makes the fallback ask every replica and use the largest
length. For writers whose PutBlock goes through the Raft log, which is the
default, datanodes apply a PutBlock only after it is committed in Ratis, and
the length of a block only grows with the applied log. The largest length among
the replicas is then the length at the most advanced replica, which is the
state ReplicationManager converges to (argued from the code).
Not covered:
* A data stream writer with
{{ozone.client.datastream.putblock.without.raft.enabled}} (off by default)
sends PutBlock outside the Raft log, so each datanode applies it on its own and
the argument above does not hold. There the largest length can be one that only
one replica has (argued, not run).
* If only lagging replicas answer during recovery, the result is still too
short, and a length below the one OM already holds for the block is still
accepted. Failing recovery in that case unless it is forced belongs in
{{LeaseRecoveryClientDNHandler}} and is not part of this patch. Requiring an
answer from all or a majority of the replicas would close the rest, at the
price of failing recovery whenever a datanode is down, which is the situation
recovery exists for.
* Until ReplicationManager has replaced the lagging replica, a reader that is
served by it gets an error for the bytes it does not have (from reading the
code, not run), because the recovered key carries block commit sequence ID 0
and the read path cannot reject that replica. Returning the sequence ID from
{{GetCommittedBlockLength}} and storing it at recovery would address that. It
needs a protocol change.
h3. Patch
[^MC-5-lease-recovery-use-largest-replica-block-length.patch], against
ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199. It also applies to master at
84f5594a6a2.
{{ContainerProtocolCalls.getMaxCommittedBlockLength}} sends
{{GetCommittedBlockLength}} to all datanodes of the pipeline with the existing
{{XceiverClientSpi.sendCommandOnAllNodes}} and returns the largest length. Both
adapters use it in the fallback. Replicas that fail or do not answer are
skipped, and one warning is logged when a replica is skipped or the lengths
differ. What changes for recoveries the bug does not affect:
* It is one request per datanode of the pipeline of the block, sent in
parallel, and all answers are awaited. For a container that is no longer open
that is every replica SCM lists, in any state. A datanode that accepts the
request and does not answer now always costs one {{ozone.client.read.timeout}}
(30s), where before it cost that only if it was asked before one that answered.
The worst case drops from one timeout per replica in a row to one.
* If no replica returns a length, {{NO_SUCH_BLOCK}} or {{CONTAINER_NOT_FOUND}}
is thrown only when every datanode that was asked answered with one of the two,
so {{LeaseRecoveryClientDNHandler}} skips a block only then. Otherwise an
{{IOException}} is thrown. Before, the error of the last datanode tried was
thrown.
* An {{UNAUTHENTICATED}} answer of a datanode no longer surfaces as
{{SCMSecurityException}}. That replica is skipped like one that is unreachable
(from the code, not run).
* EC keys do not reach the fallback (hsync is not supported for EC and the
fallback rejects non replicated keys).
Covered by two unit tests in {{TestContainerProtocolCalls}} and by
{{testBlockLengthOfLaggingReplicaIsNotUsed}} in the existing
{{TestLeaseRecovery}}, which fails without the change with a file length of 100
instead of 200. Two assertions in {{TestLeaseRecovery}} that counted the
datanodes tried one after another now count the requests that are sent. On
unmodified source those two tests ({{testGetCommittedBlockLengthTimeout}},
{{testGetCommittedBlockLengthWithException}}) fail intermittently, because
their helper {{closeLatestContainer}} closes the last container SCM lists,
which is not always the container of the file, and then the fallback is never
reached. With the patch the reproduction fails at its length assertion, because
the file is closed with 200 bytes.
Found by TLA+ model checking and code review of the lease recovery paths for
hsynced files under HDDS-15926, on commit
ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199. 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
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
