Copilot commented on code in PR #11353:
URL: https://github.com/apache/ozone/pull/11353#discussion_r4138918019
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##########
@@ -506,6 +503,25 @@ public RegisteredCommand register(
.build();
}
+ /**
+ * Replaces the record of an already registered datanode, in both the node
state and the network
+ * topology. The topology has to be updated first: {@link
NetworkTopology#update} sets the parent on
+ * the new node, and {@link NodeStateManager#updateNode} only picks it up
because the stored
+ * {@link DatanodeInfo} is copied from that node afterwards. Updating the
node state alone would
+ * leave the stale record in the topology and the stored node without a
parent.
+ *
+ * @return the refreshed node as held by the node state manager
+ */
+ private DatanodeDetails updateNodeAndTopology(DatanodeInfo oldNode,
+ DatanodeDetails datanodeDetails, LayoutVersionProto layoutInfo)
+ throws NodeNotFoundException {
+ clusterMap.update(oldNode, datanodeDetails);
+ nodeStateManager.updateNode(datanodeDetails, layoutInfo);
Review Comment:
A concurrent `DeadNodeHandler` can still undo this repair. It can observe
`DEAD` and make `contains(oldNode)` return true, then this update replaces the
topology entry, after which the handler's separate `remove(oldNode)` removes
the newly added entry by path. `updateNode` then publishes `HEALTHY` without a
transition event, and the parent assertion still passes because removal clears
the old object’s parent, not `datanodeDetails`’ parent. The node is therefore
again healthy but permanently absent from topology. Please coordinate
registration with dead-node topology removal under a shared atomic
protocol/lock (and add the corresponding interleaving test) rather than relying
on update-before-state ordering alone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
