Siyao Meng created HDDS-17620:
---------------------------------

             Summary: SCM sends another DELETE for an UNHEALTHY EC replica 
while one is pending, and the queued copy can delete a replica later rebuilt on 
that datanode
                 Key: HDDS-17620
                 URL: https://issues.apache.org/jira/browse/HDDS-17620
             Project: Apache Ozone
          Issue Type: Bug
            Reporter: Siyao Meng
         Attachments: 
MC-1-skip-repeat-delete-of-unhealthy-ec-replica-while-pending.patch

h3. Mechanism
{{ClosedWithUnhealthyReplicasHandler.handle}} sends a force DELETE for every 
UNHEALTHY replica of a CLOSED EC container whose index also has a CLOSED copy, 
on every ReplicationManager check, without looking at the container's pending 
ops. {{ECContainerReplicaCount}} ignores pending deletes of UNHEALTHY replicas, 
so the container stays HEALTHY in {{ECReplicationCheckHandler}} and reaches 
this handler again on the next pass. While the first DELETE is still queued on 
the datanode, a second one is sent. SCM keeps a single pending op per target 
and replica index ({{ContainerReplicaPendingOps.addReplica}} replaces the 
earlier one), so the report of the first delete completes the only op and the 
datanode no longer counts as holding the container 
({{ReplicationManagerUtil.getExcludedAndUsedNodes}}).

On the datanode, {{DeleteContainerCommandHandler.handleInternal}} checks only 
the command deadline and the SCM term, then 
{{ContainerController.deleteContainer}} deletes the replica with that container 
ID, whatever its replica index. If another replica of the same container is 
placed on that datanode before the second DELETE runs (EC reconstruction of 
another index, or a balancer move, since {{MoveManager.move}} also checks only 
the replicas and pending ops SCM knows of), the second DELETE removes it.

{{ECUnderReplicationHandler.checkAndRemoveUnhealthyReplica}} and 
{{DeletingContainerHandler}} already skip replicas with a delete in flight; 
this handler does not.

h3. Trigger
Default settings: monitor interval 300s, {{hdds.scm.replication.event.timeout}} 
12m, datanode offset 6m, 2 delete container threads per datanode.
# A CLOSED EC container has a CLOSED and an UNHEALTHY replica of index 2; the 
UNHEALTHY one is on datanode V. A check sends DELETE D1 to V.
# D1 waits in V's delete queue behind other delete work. The next check, 5 
minutes later, sends D2 for the same replica; it reaches V while the replica 
still exists.
# D1 runs before its datanode deadline (6 minutes after it was sent) and V's 
report completes the pending op.
# Another index of the same container (index 1) is lost, and reconstruction 
rebuilds it on V while D2 is still queued.
# D2 runs and deletes the rebuilt index 1 replica.

This needs a delete backlog of minutes on V, the loss of another index of the 
same container within the same few minutes, and V chosen as the reconstruction 
target, so it is unlikely.

h3. Impact
A replica that SCM just placed and recorded as present is deleted, for example 
an index rebuilt by reconstruction or the target copy of a balancer move. The 
container loses redundancy again until ReplicationManager repairs it. No data 
is lost on its own.

h3. Reproduction
PASS, deterministic, unmodified source. 
[^TestBugMC1RepeatedUnhealthyDeleteRemovesRebuiltReplica.java] 
({{hadoop-hdds/server-scm}}) runs the real ReplicationManager check chain and 
{{ContainerReplicaPendingOps}} with mocked node and container managers, and 
gives V a real {{DeleteContainerCommandHandler}}, {{ContainerController}}, 
{{KeyValueHandler}} and {{ContainerSet}} on a temporary volume. Commands are 
handed to V by the test, V's delete thread (one in the test) is held with 
latches, and V's report is applied to the replica map by hand. Reconstruction 
target selection is not run: the test checks that V is neither excluded nor 
used for it, then creates index 1 on V with the CreateContainer (RECOVERING) 
and CloseContainer requests that reconstruction sends. Output:
{noformat}
after second check: DELETE commands sent to V = 2, pending DELETE ops = 1
rebuilt on V: replicaIndex = 1, state = CLOSED
after the second DELETE (replicaIndex 2) ran on V: container present = false, 
expired = 0
{noformat}
A passing test means the defect is present. No mini cluster was run.

h3. Suggested fix
Skip the delete in SCM rather than on the datanode. A datanode check of the 
command's replica index would leave a replica of the same index placed later 
exposed, and {{AbstractContainerReportHandler.deleteReplica}} sends deletes 
without a replica index (0) for EC replicas, so such a check would have to 
treat 0 as matching any index.

The expiry path of the same datanode behaviour (a DELETE resent after its op 
expired, HDDS-12127) is HDDS-16897, whose attached patch covers it. It does not 
cover this case, because here the replica is still present when both commands 
arrive.

h3. Patch
[^MC-1-skip-repeat-delete-of-unhealthy-ec-replica-while-pending.patch], against 
e4ae41fc4047c99af983020b4f33bd297c9d0ac5. It also applies to master at 
3fc2387040f.

{{ClosedWithUnhealthyReplicasHandler.handle}} does not send a DELETE for an 
UNHEALTHY replica whose datanode already has a pending DELETE for the 
container. The container is still reported {{UNHEALTHY_OVER_REPLICATED}}. A 
pending DELETE that expires is still resent by 
{{ReplicationManager.opCompleted}}, so the replica is retried every 12 minutes 
by the existing resend instead of every monitor pass.

Covered by {{testUnhealthyECReplicaWithPendingDeleteIsNotDeletedAgain}} in the 
existing {{TestReplicationManager}}: a second check sends no DELETE for the 
replica with a pending delete and still deletes another UNHEALTHY replica 
without one. Without the change it fails (two commands sent). With it the 
replication manager, scenario, EC handler, health check and pending ops suites 
pass (213 tests) and checkstyle is clean. With the patch the reproduction 
fails, because the second check sends no DELETE.

Found by TLA+ model checking and code review of the SCM EC container 
replication and placement repair paths under HDDS-15926, on commit 
e4ae41fc4047c99af983020b4f33bd297c9d0ac5. 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]

Reply via email to