[
https://issues.apache.org/jira/browse/HDDS-17225?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17225:
------------------------------
Attachment:
CR-1-block-decommission-on-combined-under-replicated-states.patch
> Decommission completes while a container on the node is under replicated but
> recorded under a combined state such as UNHEALTHY_UNDER_REPLICATED
> -----------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17225
> URL: https://issues.apache.org/jira/browse/HDDS-17225
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> CR-1-block-decommission-on-combined-under-replicated-states.patch,
> TestBugCR1CombinedUnderReplicatedStates.java
>
>
> h3. Mechanism
> {{DatanodeAdminMonitorImpl.checkContainersReplicatedOnNode}} runs
> {{ReplicationManager.checkContainerStatus}} for each container on the node
> and treats the container as replicated when
> {{report.getStat(ContainerHealthState.UNDER_REPLICATED) == 0}}.
> {{ReplicationManagerReport.getStat}} returns the counter of exactly that name.
> HDDS-14119 changed several handlers that used to record plain
> {{UNDER_REPLICATED}} so that they record only a combined state, and the
> monitor line was only renamed to the new enum:
> * {{QuasiClosedStuckReplicationCheck}}:
> {{QUASI_CLOSED_STUCK_UNDER_REPLICATED}}
> * {{RatisUnhealthyReplicationCheckHandler}} and
> {{VulnerableUnhealthyReplicasHandler}}: {{UNHEALTHY_UNDER_REPLICATED}}
> * {{ECReplicationCheckHandler}}, unrecoverable container with offline
> indexes: {{MISSING_UNDER_REPLICATED}} or {{UNHEALTHY_UNDER_REPLICATED}}. The
> comment there still says "the decommission monitor needs to wait for an extra
> copy to be made of the offline replica before decommission can complete".
> A container recorded under one of these states no longer holds the node. The
> existing monitor tests for these cases
> ({{testDecommissionWaitsForUnhealthyReplicaToReplicateNewRM}},
> {{testDecommissionWaitsForUnhealthyReplicaWithUniqueOriginToReplicateNewRM}},
> {{testDecommissionNodeWithUnrecoverableECContainer}}) still pass because
> {{DatanodeAdminMonitorTestUtil.mockCheckContainerState}} injects plain
> {{UNDER_REPLICATED}}.
> h3. Trigger
> An admin decommissions a datanode, or puts it into maintenance, while it
> holds a replica of a container such as:
> * a QUASI_CLOSED container whose UNHEALTHY replica on the node has a unique
> origin or the highest BCSID, next to QUASI_CLOSED replicas of one origin (the
> HDDS-10113 case);
> * a CLOSED RATIS/THREE container whose only replicas are UNHEALTHY;
> * an unrecoverable EC container with an index only on the node.
> After the pipelines on the node are closed, the next monitor run completes
> the workflow.
> h3. Impact
> The node becomes DECOMMISSIONED (or IN_MAINTENANCE) and is no longer tracked,
> while it still holds the replica that the check was meant to protect.
> Replication Manager still queues these containers and can copy from the node
> while it is up, so the data is lost only if the operator removes the node (or
> a further failure happens) before that copy exists. In the highest BCSID case
> the replica on the node can be the only copy of the latest blocks.
> ozone-2.2.0 and ozone-2.2.1 contain HDDS-14119 and the same monitor line
> (read from the tagged source, not run on a release).
> h3. Reproduction
> PASS on unmodified source, deterministic.
> [^TestBugCR1CombinedUnderReplicatedStates.java] ({{hadoop-hdds/server-scm}})
> runs the monitor against a real {{ReplicationManager}} with its real check
> chain (no background threads), {{SimpleMockNodeManager}}, a stub
> {{ContainerManager}} and a placement policy that always validates. For each
> layout it first asserts which state the real chain records, then runs the
> monitor twice:
> * control, CLOSED RATIS/THREE with one replica on the node and two others:
> {{UNDER_REPLICATED=1}}, the node stays DECOMMISSIONING;
> * HDDS-10113 unique origin case: {{QUASI_CLOSED_STUCK_UNDER_REPLICATED=1}},
> DECOMMISSIONED;
> * all UNHEALTHY RATIS/THREE: {{UNHEALTHY_UNDER_REPLICATED=1}}, DECOMMISSIONED;
> * EC 3-2 with only indexes 1 (on the node) and 2:
> {{MISSING_UNDER_REPLICATED=1}}, DECOMMISSIONED.
> A passing test means the defect is present. The maintenance path uses the
> same check and was not run separately. The
> {{VulnerableUnhealthyReplicasHandler}} path was covered only through the
> mocked monitor test below.
> h3. Patch
> [^CR-1-block-decommission-on-combined-under-replicated-states.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 20d95ed37fe.
> The monitor treats a container as replicated only when {{UNDER_REPLICATED}},
> {{UNHEALTHY_UNDER_REPLICATED}}, {{MISSING_UNDER_REPLICATED}} and
> {{QUASI_CLOSED_STUCK_UNDER_REPLICATED}} are all zero. That restores the
> behaviour from before HDDS-14119 for every handler. The other combined states
> are over replicated or missing, and {{MISSING}} alone did not block before
> either (HDDS-7666), so they are left out. The grouping is a constant in the
> monitor, its only consumer. It could instead be a method on
> {{ContainerHealthState}} if reviewers prefer to keep it next to the combined
> constants.
> Covered by two parameterized tests in the existing
> {{TestDatanodeAdminMonitor}}:
> {{testDecommissionWaitsForUnderReplicatedHealthState}} (the four states
> block) and {{testDecommissionNotBlockedByOtherHealthState}} (the other twelve
> states do not). Without the change the three combined state cases fail with
> "expected: <DECOMMISSIONING> but was: <DECOMMISSIONED>". With it
> {{TestDatanodeAdminMonitor}} (33), {{TestNodeDecommissionMetrics}} and
> {{TestNodeDecommissionManager}} pass, as does the mini cluster suite
> {{TestDecommissionAndMaintenance}} (9 tests), checkstyle is clean, and the
> three bug cases of the reproduction fail because the node stays
> DECOMMISSIONING.
> Found by code review of the SCM datanode decommission and maintenance admin
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-14119 (introduced the combined states), HDDS-16446 (quasi
> closed stuck replica counting, a different defect in the same handler). 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]
