This is an automated email from the ASF dual-hosted git repository.
jojochuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new ecafca6f6f6 HDDS-16630. Update the network topology when a datanode
re-registers with new ports or a new version (#11353)
ecafca6f6f6 is described below
commit ecafca6f6f6d29b8f7e6c889d89a5aa6a11cdfce
Author: Andrey Yarovoy <[email protected]>
AuthorDate: Wed Sep 30 12:13:13 2026 -0400
HDDS-16630. Update the network topology when a datanode re-registers with
new ports or a new version (#11353)
Generated-by: Claude Code (Claude Opus 5)
---
.../hadoop/hdds/scm/node/SCMNodeManager.java | 28 +++++++---
.../hadoop/hdds/scm/node/TestSCMNodeManager.java | 63 ++++++++++++++++++++--
2 files changed, 80 insertions(+), 11 deletions(-)
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
index 400b01ebdef..92111687361 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
@@ -476,10 +476,7 @@ public RegisteredCommand register(
if (updateDnsToDnIdMap(oldNode.getHostName(), oldNode.getIpAddress(),
hostName, ipAddress, dnId)) {
LOG.info("Updating datanode from {} to {}", oldNode,
datanodeDetails);
- clusterMap.update(oldNode, datanodeDetails);
- nodeStateManager.updateNode(datanodeDetails, layoutInfo);
- DatanodeDetails dn = nodeStateManager.getNode(datanodeDetails);
- Preconditions.checkState(dn.getParent() != null);
+ DatanodeDetails dn = updateNodeAndTopology(oldNode, datanodeDetails,
layoutInfo);
processNodeReport(datanodeDetails, nodeReport);
LOG.info("Updated datanode to: {}", dn);
scmNodeEventPublisher.fireEvent(SCMEvents.NODE_ADDRESS_UPDATE, dn);
@@ -487,12 +484,12 @@ public RegisteredCommand register(
LOG.info("Update the version for registered datanode {}, " +
"oldVersion = {}, newVersion = {}.",
datanodeDetails, oldNode.getVersion(),
datanodeDetails.getVersion());
- nodeStateManager.updateNode(datanodeDetails, layoutInfo);
+ updateNodeAndTopology(oldNode, datanodeDetails, layoutInfo);
} else if (oldNode.portsChanged(datanodeDetails)) {
// Refresh the stored node when its port set changes
LOG.info("Updating ports for registered datanode {}: {} -> {}",
datanodeDetails, oldNode.getPorts(), datanodeDetails.getPorts());
- nodeStateManager.updateNode(datanodeDetails, layoutInfo);
+ updateNodeAndTopology(oldNode, datanodeDetails, layoutInfo);
}
} catch (NodeNotFoundException e) {
LOG.error("Cannot find datanode {} from nodeStateManager",
@@ -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);
+ final DatanodeDetails dn = nodeStateManager.getNode(datanodeDetails);
+ Preconditions.checkState(dn.getParent() != null);
+ return dn;
+ }
+
/**
* Add an entry to the dnsToUuidMap, which maps hostname / IP to the DNs
* running on that host. As each address can have many DNs running on it,
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
index 868ca469d9c..0cc8818f96f 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
@@ -379,32 +379,85 @@ private void registerNode(SCMNodeManager nodeManager,
DatanodeDetails dn) {
/**
* A datanode that re-registers with the same identity but now exposes the
* RATIS_DATASTREAM port (e.g. Ratis DataStream was enabled) must have its
- * stored record refreshed so the new port is visible (HDDS-15799).
+ * stored record refreshed so the new port is visible (HDDS-15799), in the
+ * network topology as well as in the node state (HDDS-16630).
*/
@Test
public void testRegisterRefreshesPortsOnPortChange()
throws IOException, AuthenticationException {
try (SCMNodeManager nodeManager = createNodeManager(getConf())) {
final UUID uuid = UUID.randomUUID();
+ final DatanodeID dnId = datanodeWithoutDatastream(uuid).build().getID();
+ final NetworkTopology clusterMap = scm.getClusterMap();
// First registration: streaming disabled, no RATIS_DATASTREAM port.
registerNode(nodeManager, datanodeWithoutDatastream(uuid).build());
- DatanodeDetails stored = nodeManager.getNode(
- datanodeWithoutDatastream(uuid).build().getID());
+ DatanodeDetails stored = nodeManager.getNode(dnId);
assertFalse(stored.hasPort(DatanodeDetails.Port.Name.RATIS_DATASTREAM));
+ assertFalse(nodeInTopology(clusterMap, stored)
+ .hasPort(DatanodeDetails.Port.Name.RATIS_DATASTREAM));
// Re-registration (same id/ip/host/version) now exposing the port.
registerNode(nodeManager, datanodeWithoutDatastream(uuid)
.addPort(DatanodeDetails.newPort(
DatanodeDetails.Port.Name.RATIS_DATASTREAM, 9855))
.build());
- stored = nodeManager.getNode(
- datanodeWithoutDatastream(uuid).build().getID());
+ stored = nodeManager.getNode(dnId);
assertTrue(stored.hasPort(DatanodeDetails.Port.Name.RATIS_DATASTREAM),
"stored node should be refreshed with the RATIS_DATASTREAM port");
+ assertEquals(1, clusterMap.getNumOfLeafNode(""));
+ assertTrue(clusterMap.contains(stored),
+ "refreshed node should still be in the network topology");
+ assertTrue(nodeInTopology(clusterMap, stored)
+ .hasPort(DatanodeDetails.Port.Name.RATIS_DATASTREAM),
+ "network topology should hold the refreshed node, not the stale
one");
+ }
+ }
+
+ /**
+ * A datanode re-registering with a new version must be refreshed in the
network topology too,
+ * including when it was previously removed from the topology, as {@link
DeadNodeHandler} does
+ * for a dead node (HDDS-16630).
+ */
+ @Test
+ public void testRegisterUpdatesTopologyOnVersionChange()
+ throws IOException, AuthenticationException {
+ try (SCMNodeManager nodeManager = createNodeManager(getConf())) {
+ final UUID uuid = UUID.randomUUID();
+ final DatanodeID dnId = datanodeWithoutDatastream(uuid).build().getID();
+ final NetworkTopology clusterMap = scm.getClusterMap();
+
+ registerNode(nodeManager,
+ datanodeWithoutDatastream(uuid).setVersion("1.0.0").build());
+
+ // Re-registration with a new version, everything else unchanged.
+ registerNode(nodeManager,
+ datanodeWithoutDatastream(uuid).setVersion("2.0.0").build());
+ DatanodeDetails stored = nodeManager.getNode(dnId);
+ assertEquals("2.0.0", stored.getVersion());
+ assertEquals(1, clusterMap.getNumOfLeafNode(""));
+ assertTrue(clusterMap.contains(stored),
+ "refreshed node should still be in the network topology");
+ assertEquals("2.0.0", nodeInTopology(clusterMap, stored).getVersion(),
+ "network topology should hold the refreshed node, not the stale
one");
+
+ // A node the DeadNodeHandler dropped from the topology must re-enter it
on re-registration.
+ clusterMap.remove(stored);
+ assertEquals(0, clusterMap.getNumOfLeafNode(""));
+ registerNode(nodeManager,
+ datanodeWithoutDatastream(uuid).setVersion("3.0.0").build());
+ stored = nodeManager.getNode(dnId);
+ assertEquals(1, clusterMap.getNumOfLeafNode(""));
+ assertTrue(clusterMap.contains(stored),
+ "node removed from the topology should be added back on
re-registration");
}
}
+ private static DatanodeDetails nodeInTopology(NetworkTopology clusterMap,
+ DatanodeDetails node) {
+ return (DatanodeDetails) clusterMap.getNode(node.getNetworkFullPath());
+ }
+
/**
* Re-registering a datanode with an unchanged port set must not disturb the
* stored record (the port-refresh branch is skipped).
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
