Siyao Meng created HDDS-16898:
---------------------------------
Summary: ReplicationManager resends an expired force DELETE to the
last known replica of a container after the other replicas are lost
Key: HDDS-16898
URL: https://issues.apache.org/jira/browse/HDDS-16898
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-1-do-not-resend-expired-force-delete-of-last-replica.patch,
TestBugMC1ExpiredDeleteResentToLastReplica.java
h3. Mechanism
Since HDDS-12127, {{ContainerReplicaPendingOps.removeExpiredEntries}} keeps an
expired DELETE op and {{ReplicationManager.opCompleted}} resends the same
{{DeleteContainerCommand}} with a new deadline. The resend does not look at the
replicas the container has now. Whether the delete is safe was checked once,
when it was first sent: for a balancer move in
{{MoveManager.handleSuccessfulAdd}}, for ReplicationManager in its over
replication and unhealthy replica handlers. Nothing cancels the pending DELETE
when the other replicas go away, so after {{DeadNodeHandler}} has removed the
replicas of dead nodes, SCM still resends a force DELETE to the only replica it
knows of. While that op is pending, {{RatisUnderReplicationHandler.getSources}}
excludes its target as a replication source, so ReplicationManager cannot copy
the container from it either.
On the datanode, {{DeleteContainerCommandHandler.handleInternal}} checks only
the command deadline and the SCM term, and {{KeyValueHandler.deleteInternal}}
skips the open and non empty checks when {{force}} is set. A unit level run
during review confirmed that a force {{DeleteContainerCommand}} passed to
{{DeleteContainerCommandHandler.handle}} removes a CLOSED container with blocks
from the container set and deletes its directory; the existing
{{TestKeyValueHandler#testDeleteDecrementsVolumeUsedSpace}} also force deletes
a CLOSED container with blocks.
h3. Trigger
# A CLOSED RATIS/THREE container has replicas on A, B and C. The balancer moves
it from A to D.
# D reports its replica, the move check passes and MoveManager sends a force
DELETE to A.
# A stays alive but does not run that DELETE, for example because the delete
waited longer than {{hdds.datanode.delete.container.timeout}} (60s) for the
container lock (the datanode then skips it and relies on the SCM resend), or
because its delete container queue is full.
# B, C and D become dead and their replicas are removed. SCM knows only A, the
container is under replicated, and no replication command is sent because A has
a pending delete.
# The deadline passes. MoveManager ends the move with
{{DELETION_FAIL_TIME_OUT}} and ReplicationManager resends the force DELETE to A
with a fresh deadline.
The resend repeats every {{hdds.scm.replication.event.timeout}} (12 minutes)
for as long as A is alive and has not deleted the replica, so the other nodes
do not have to die within one move timeout. They do have to die while A keeps
failing to run the delete.
h3. Impact
If A runs the resent command, the last copy SCM knows of is deleted and the
container has no known replica. The data then exists only on the dead nodes and
is lost if they do not come back. For a RATIS container this needs three
replica holders to become dead while a fourth stays alive but does not run
deletes, so it is unlikely, but the delete is issued by SCM itself and nothing
stops it.
For EC the condition is per replica index and much easier to meet: MoveManager
also moves EC replicas with a force delete of the source index, so only the
move target has to die after the source delete was sent. The resend then
deletes the last copy of that index, which must be reconstructed. Redundancy
drops, but there is no data loss on its own.
A weaker RATIS case is not changed by the patch: if only B and C die, the
resend leaves D as the only replica (1 of 3). The container stays recoverable
from D.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugMC1ExpiredDeleteResentToLastReplica.java] ({{hadoop-hdds/server-scm}})
uses the real {{ContainerManagerImpl}}, {{ContainerReplicaPendingOps}},
{{ReplicationManager}}, {{MoveManager}} and {{DeadNodeHandler}}, subscribed in
the same order as in {{StorageContainerManager}}. Only the node manager (node
status and command sink), the placement policy and the SCM context are mocks. A
not running the first DELETE is modelled by never delivering it, and the
scrubber is {{removeExpiredEntries}} after the clock passes the move timeout.
Output on the unmodified source:
{noformat}
move result: DELETION_FAIL_TIME_OUT
replicas known to SCM: 1 (only A), pending DELETE ops on A: 1
commands sent by the scrubber run: [(<A>,deleteContainerCommand: ...
containerID: 1, replicaIndex: 0, force: true)]
{noformat}
A passing test means the defect is present. No datanode runs in this test; the
datanode side is covered by the run described under Mechanism.
h3. Suggested fix
Before resending a force DELETE, check that another copy of the same replica
index can still serve as a replication source, and drop the pending delete if
none can. The patch counts as a copy only a replica that is not UNHEALTHY, is
on a HEALTHY node (the same node check
{{RatisUnderReplicationHandler.getSources}} uses for sources) and is not itself
pending deletion. A copy on a STALE node therefore does not count, which also
protects A during the 10 minutes before the failed nodes are declared dead. If
such a peer comes back, the container is over replicated again and
ReplicationManager issues a fresh, checked delete.
A broader recheck of the container health before each resend would also cover
the 1 of 3 case, but would change the retry of deletes that ReplicationManager
issues on purpose while a container is under replicated
({{RatisUnderReplicationHandler.removeUnhealthyReplicaIfPossible}}), so it is
not part of this patch.
h3. Patch
[^MC-1-do-not-resend-expired-force-delete-of-last-replica.patch], against
b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
6818707698c.
Before resending an expired force DELETE, {{ReplicationManager.opCompleted}}
checks for another usable copy of the same index as described above. If there
is none, it logs a warning and removes the pending op instead of resending, so
the replica becomes a replication source again. Non force deletes (empty and
DELETING containers) are resent as before. The op is removed with a new
{{ContainerReplicaPendingOps.removeExpiredOp}}, which does not notify
subscribers: {{removeOp}} would report the op as completed, and because
ReplicationManager is subscribed before MoveManager, MoveManager would then
finish the move as {{COMPLETED}} instead of {{DELETION_FAIL_TIME_OUT}}.
Covered in the existing {{TestReplicationManager}} by
{{testExpiredDeleteOfLastReplicaIsResentOnlyIfNotForce}} (the force delete is
dropped and later subscribers see only the expiry; a non force delete is still
resent), {{testExpiredForceDeleteIsNotResentWhenOtherCopyIsNotUsable}} (the
other copy is UNHEALTHY, or CLOSED on a STALE node),
{{testExpiredForceDeleteOfLastCopyOfEcIndexIsNotResent}} and
{{testExpiredForceDeletesKeepOneReplicaWhenAllRemainingArePendingDelete}};
{{testPendingOpExpiry}} now registers the replicas it relies on. Without the
change every force case fails because the DELETE is resent. With it the
replication manager, pending ops and balancer unit suites pass and checkstyle
is clean. With the patch the reproduction fails: no command is sent, the
pending DELETE is gone and the move result is still {{DELETION_FAIL_TIME_OUT}}.
The resend was added by HDDS-12127. HDDS-16447 proposes another precondition
for the same resend in {{ReplicationManager.opCompleted}} (resend only while
the target replica is still present). The two checks are independent, but both
patches change the same lines and should be combined when applied. HDDS-16897
is a separate defect enabled by the same resend (a resent copy that outlives
its pending op and deletes a newer replica on the datanode); it needs a
datanode side fix.
Found by TLA+ model checking and code review of the SCM container move and
balancer paths under HDDS-15926, on commit
b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. 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]
