Siyao Meng created HDDS-17623:
---------------------------------
Summary: Mis-replication of a container is not repaired while one
of its replicas is on a datanode that is DEAD in maintenance
Key: HDDS-17623
URL: https://issues.apache.org/jira/browse/HDDS-17623
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
CR-1-resolve-rack-of-replica-on-node-outside-topology.patch,
TestBugCR1DeadMaintenanceMisReplication.java,
TestBugCR7ReregisteredNodeReplicaRack.java
h3. Mechanism
{{SCMCommonPlacementPolicy.getPlacementGroup}} finds a datanode's rack by
walking up from the {{DatanodeDetails}} object in the network topology. It
returns null for a datanode that is no longer in the topology. Two of its
callers do not handle that:
* {{replicasToCopyToFixMisreplication}} groups the replicas by rack with
{{Collectors.groupingBy}}, which throws {{NullPointerException: element cannot
be mapped to a null key}}.
* {{replicasToRemoveToFixOverreplication}} adds the racks to a
{{PriorityQueue}}, which throws a {{NullPointerException}} on null.
{{validateContainerPlacement}} already handles a null rack: HDDS-8950 drops it,
and HDDS-13762 counts a node that is DEAD and IN_MAINTENANCE on the rack of its
network location, because it is expected to come back.
{{MisReplicationHandler}} and {{ECOverReplicationHandler}} throw,
{{UnhealthyReplicationProcessor.processAll}} logs the exception at ERROR and
requeues the container, and no command is sent.
A replica's datanode is outside the topology in two cases:
# *DEAD in maintenance.* {{NodeStateManager}} removes a node from the topology
when it becomes DEAD (since HDDS-16623, before that {{DeadNodeHandler}} did),
and {{DeadNodeHandler}} keeps the replicas of a node that is IN_MAINTENANCE.
The replicas hold the same {{DatanodeInfo}} object, which now has no parent.
This is the normal maintenance workflow: the node is put in maintenance and
then shut down.
# *Re-registration with changed details.* When a registered datanode registers
again with a new address, a new version or new ports,
{{SCMNodeManager.updateNodeAndTopology}} calls {{NetworkTopology.update}},
which removes the stored {{DatanodeInfo}} from the tree and clears its parent,
and stores a new object. Replicas built from earlier reports still hold the old
object until the full container report sent with the registration is processed.
h3. Trigger
Case 1: a container that is mis-replicated for another reason (for example two
replicas on one rack) has a replica on a datanode that enters maintenance and
is then stopped. {{ozone admin datanode maintenance}} without {{--end}} sets no
end time, so the node stays DEAD and IN_MAINTENANCE until an operator brings it
back or ends maintenance. This was run through
{{NodeDecommissionManager.startMaintenanceNodes}} and the real health check
(see Reproduction).
Case 2: a datanode restarts with a changed address, version (rolling upgrade)
or port set, for example after enabling Ratis DataStream.
h3. Impact
* Case 1: every Replication Manager pass queues the container, the
mis-replication handler throws and no copy is sent, for as long as the node
stays DEAD in maintenance. The container keeps its reduced rack spread for that
time, and one ERROR with a stack trace is logged per container and pass. The
same handler is used for Ratis containers through
{{RatisMisReplicationHandler}} (argued from the code, only EC was run).
Over-replication is not affected in this case, because
{{ECOverReplicationHandler}} only passes replicas on healthy in-service nodes
to the placement policy.
* Case 2: until the registration report is processed, a container whose
replicas are on distinct racks is flagged mis-replicated, and the EC
mis-replication and over-replication handlers throw for it. No wrong command is
sent (target selection resolves datanodes through {{NodeManager}}), and the
report normally arrives within seconds, so the effect is a short delay and
ERROR logs. It lasts longer only if the registration report is lost (argued,
not run).
Releases from 1.4.0 have case 1 by code reading (not run on a release). In
1.4.0 to 2.1.x, before HDDS-13762, {{validateContainerPlacement}} also dropped
the rack of a DEAD maintenance node, so an EC container with a replica on such
a node could be flagged mis-replicated for that reason alone and then hit the
exception (argued from the code, not run).
h3. Reproduction
Both classes run in {{hadoop-hdds/server-scm}}, pass on unmodified source when
the bug is present, and have the target path and Maven command in the class
comment. They use a real SCM ({{HddsTestUtils.getScm}}) with a rack mapping,
real {{SCMNodeManager}} registration, the real {{ContainerReportHandler}}, the
EC placement policy SCM builds (rack scatter), and the real check handler and
EC handlers with SCM's own {{ReplicationManager}}. {{ContainerManager}} is a
map backed stub that stores the replicas the report handler creates.
* [^TestBugCR1DeadMaintenanceMisReplication.java]: rs-3-2 container with index
1 and 2 on one rack, and index 3 and 4 on another. The datanode with index 3 is
put in maintenance with {{NodeDecommissionManager.startMaintenanceNodes}} (no
end time) and made DEAD by the real health check, which removes it from the
topology. The admin monitor step to IN_MAINTENANCE and the datanode heartbeat
that reports it are done with the calls those components make. The container is
queued as mis-replicated, {{ECMisReplicationHandler}} throws
{{NullPointerException: element cannot be mapped to a null key}} from
{{replicasToCopyToFixMisreplication}}, and no command is queued. With the patch
the handler sends two replicate commands to the free racks; the one for index 4
is sent because the DEAD node now counts on the rack it shares with index 4.
* [^TestBugCR7ReregisteredNodeReplicaRack.java]: one datanode per rack, a
container on 5 racks and one with an extra copy of index 1. One datanode
registers again with an extra port. Before its registration report is
processed, the replica's datanode has no parent while {{NodeManager}}'s has
one, the container on 5 racks is flagged mis-replicated, and
{{ECMisReplicationHandler}} and {{ECOverReplicationHandler}} throw. After the
registration report all of this is gone. With the patch the container is not
flagged.
h3. Patch
[^CR-1-resolve-rack-of-replica-on-node-outside-topology.patch], against
e4ae41fc4047c99af983020b4f33bd297c9d0ac5. It also applies to master at
3fc2387040f.
* {{getPlacementGroup}}: when the given object has no parent, look up the rack
of the datanode object {{NodeManager}} holds now, as {{validateDatanodes}}
already does for target selection. This covers case 2 for all three methods.
Nothing changes when the object is in the topology.
* {{replicasToCopyToFixMisreplication}}: group replicas by the rack path that
{{validateContainerPlacement}} uses (moved into a helper,
{{getPlacementGroupPath}}), so a DEAD maintenance node counts on the rack of
its network location there too. A replica whose rack is still unknown (a DEAD
node that is not in maintenance, before {{DeadNodeHandler}} removes its
replicas) is not counted on any rack, as in {{validateContainerPlacement}}. It
cannot be a copy source anyway. Target selection still does not know the rack
of a DEAD maintenance node, so it can pick a target on that rack, which the
check after the copy rejects, and repair may take more than one pass.
* {{replicasToRemoveToFixOverreplication}} is not changed: its candidates are
on healthy in-service nodes, which are in the topology, and case 2 is covered
by the first change.
Covered by {{testReplicasToCopyWithDeadNodeOutOfTopology}} (the DEAD node
shares a rack with a healthy replica: when it is IN_MAINTENANCE that replica
must be copied, when it is IN_SERVICE nothing) and
{{testReplicaPlacementAfterReregistration}} in the existing
{{TestSCMContainerPlacementRackScatter}}, which use a real
{{NetworkTopologyImpl}}. Without the change the first fails with the
{{NullPointerException}} above and the second with "expected: 0 but was: 1"
(mis-replication count of a container on 5 racks). Each of the two changes is
needed for one of the tests; a variant that only skips replicas of unknown rack
also fails the first, because it copies nothing in the IN_MAINTENANCE case.
With the change, the placement policy, pipeline placement, Replication Manager
handler and node manager suites in {{hadoop-hdds/server-scm}} pass (702 tests)
and checkstyle is clean.
Found by code review of the SCM EC container replication and placement repair
paths, as part of the TLA+ verification effort under HDDS-15926, on commit
e4ae41fc4047c99af983020b4f33bd297c9d0ac5. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-8950 and HDDS-13762 (the same null rack in
{{validateContainerPlacement}}), HDDS-13544 (a DEAD maintenance node passed as
a used node), HDDS-16633 (a datanode missing from the topology after
re-registering while it is removed as DEAD), HDDS-17363 (the over-replication
handler does not count a maintenance replica when it picks the copy to delete).
With the patch a container of case 1 now gets copies, and with some rack orders
it can then enter the copy then delete loop that HDDS-17363 describes (argued
from the code, not run), so the two should be looked at together. The open pull
request for HDDS-17381 rewrites the same part of
{{validateContainerPlacement}}, so one of the two will need a rebase. 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]
