[
https://issues.apache.org/jira/browse/HDDS-16898?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16898:
------------------------------
Attachment: TestBugMC1ExpiredDeleteResentToLastReplica.java
> 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
> Priority: Major
> 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]
