You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by iv...@apache.org on 2026/09/29 01:19:51 UTC
(ozone) branch master updated: HDDS-16623. Datanode can be missing from NetworkTopology after it recovers from DEAD (#11343)
This is an automated email from the ASF dual-hosted git repository.
ivandika3 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 f39a349e90d HDDS-16623. Datanode can be missing from NetworkTopology after it recovers from DEAD (#11343)
f39a349e90d is described below
commit f39a349e90d2e3832cf769b4a543993387dd2eb6
Author: Peter Lee <pe...@gmail.com>
AuthorDate: Tue Sep 29 09:19:44 2026 +0800
HDDS-16623. Datanode can be missing from NetworkTopology after it recovers from DEAD (#11343)
---
.../hadoop/hdds/scm/node/DeadNodeHandler.java | 24 ---
.../hdds/scm/node/HealthyReadOnlyNodeHandler.java | 15 --
.../hadoop/hdds/scm/node/NodeStateManager.java | 30 ++++
.../hadoop/hdds/scm/node/SCMNodeManager.java | 2 +-
.../hadoop/hdds/scm/node/TestDeadNodeHandler.java | 181 +--------------------
.../hadoop/hdds/scm/node/TestNodeStateManager.java | 85 +++++++++-
.../apache/hadoop/hdds/scm/node/TestQueryNode.java | 23 +++
7 files changed, 145 insertions(+), 215 deletions(-)
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java
index da57666cb30..48340daa84d 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DeadNodeHandler.java
@@ -19,7 +19,6 @@
import static org.apache.hadoop.hdds.scm.events.SCMEvents.CLOSE_CONTAINER;
-import com.google.common.base.Preconditions;
import jakarta.annotation.Nullable;
import java.io.IOException;
import java.util.List;
@@ -32,7 +31,6 @@
import org.apache.hadoop.hdds.scm.container.ContainerManager;
import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
import org.apache.hadoop.hdds.scm.events.SCMEvents;
-import org.apache.hadoop.hdds.scm.net.NetworkTopology;
import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
import org.apache.hadoop.hdds.scm.pipeline.PipelineManager;
import org.apache.hadoop.hdds.scm.pipeline.PipelineNotFoundException;
@@ -126,28 +124,6 @@ public void onMessage(final DatanodeDetails datanodeDetails,
if (deletedBlockLog != null && !isNodeInMaintenance) {
deletedBlockLog.onDatanodeDead(datanodeDetails.getID());
}
-
- // Only remove from topology if the node is still DEAD. Between the time
- // the DEAD_NODE event was fired and now, the node may have been
- // resurrected (DEAD -> HEALTHY_READONLY) via a heartbeat. Removing a
- // resurrected node from the topology would leave it reachable but
- // invisible to the placement policy.
- currentStatus = nodeManager.getNodeStatus(datanodeDetails);
- if (currentStatus.getHealth() == HddsProtos.NodeState.DEAD) {
- NetworkTopology nt = nodeManager.getClusterNetworkTopologyMap();
- if (nt.contains(datanodeDetails)) {
- nt.remove(datanodeDetails);
- DatanodeDetails node = nodeManager.getNode(datanodeDetails.getID());
- //make sure after DN is removed from topology,
- //DatanodeDetails instance returned from nodeStateManager has no parent.
- if (node != null) {
- Preconditions.checkState(node.getParent() == null);
- }
- }
- } else {
- LOG.info("Skipping topology removal for dead node {} whose current " +
- "state is {}", datanodeDetails, currentStatus.getHealth());
- }
} catch (NodeNotFoundException ex) {
// This should not happen, we cannot get a dead node event for an
// unregistered datanode!
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/HealthyReadOnlyNodeHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/HealthyReadOnlyNodeHandler.java
index b7ae98bb84a..73224aef109 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/HealthyReadOnlyNodeHandler.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/HealthyReadOnlyNodeHandler.java
@@ -18,11 +18,9 @@
package org.apache.hadoop.hdds.scm.node;
import java.io.IOException;
-import java.util.Objects;
import java.util.Set;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
-import org.apache.hadoop.hdds.scm.net.NetworkTopology;
import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.hdds.scm.pipeline.PipelineManager;
@@ -95,18 +93,5 @@ public void onMessage(DatanodeDetails datanodeDetails,
"datanode {}: ", pipelineID, datanodeDetails, ex);
}
}
-
- // Always ensure the node is in the topology. Using unconditional add
- // rather than a contains-then-add check to avoid a race with
- // DeadNodeHandler, which may remove the node between the check and
- // the add. InnerNodeImpl.add() is idempotent for existing nodes.
- NetworkTopology nt = nodeManager.getClusterNetworkTopologyMap();
- nt.add(datanodeDetails);
- DatanodeDetails node = nodeManager.getNode(datanodeDetails.getID());
- if (node != null) {
- // make sure after DN is added back into topology, DatanodeDetails
- // instance returned from nodeStateManager has parent correctly set.
- Objects.requireNonNull(node.getParent(), "Parent == null");
- }
}
}
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
index 9539379fd84..208d40f8ed7 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
@@ -51,6 +51,7 @@
import org.apache.hadoop.hdds.scm.container.ContainerID;
import org.apache.hadoop.hdds.scm.events.SCMEvents;
import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.net.NetworkTopology;
import org.apache.hadoop.hdds.scm.node.states.Node2PipelineMap;
import org.apache.hadoop.hdds.scm.node.states.NodeAlreadyExistsException;
import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
@@ -104,6 +105,10 @@ public class NodeStateManager implements Runnable, Closeable {
* Used for publishing node state change events.
*/
private final EventPublisher eventPublisher;
+ /**
+ * Network topology of the cluster. DEAD nodes are removed from it.
+ */
+ private final NetworkTopology clusterMap;
/**
* Maps the event to be triggered when a node state us updated.
*/
@@ -161,16 +166,19 @@ public class NodeStateManager implements Runnable, Closeable {
*
* @param conf Configuration
* @param eventPublisher event publisher
+ * @param clusterMap network topology of the cluster
* @param layoutManager Layout version manager
*/
public NodeStateManager(ConfigurationSource conf,
EventPublisher eventPublisher,
+ NetworkTopology clusterMap,
LayoutVersionManager layoutManager,
SCMContext scmContext) {
this.layoutVersionManager = layoutManager;
this.nodeStateMap = new NodeStateMap();
this.node2PipelineMap = new Node2PipelineMap();
this.eventPublisher = eventPublisher;
+ this.clusterMap = clusterMap;
this.state2EventMap = new HashMap<>();
initialiseState2EventMap();
Set<NodeState> finalStates = new HashSet<>();
@@ -949,6 +957,7 @@ private void updateNodeState(DatanodeInfo node, Predicate<Long> condition,
getNextState(status.getHealth(), lifeCycleEvent);
NodeStatus newStatus =
nodeStateMap.updateNodeHealthState(node.getID(), newHealthState);
+ updateClusterMap(node, status, newStatus);
fireHealthStateEvent(newStatus.getHealth(), node);
}
} catch (InvalidStateTransitionException e) {
@@ -958,6 +967,27 @@ private void updateNodeState(DatanodeInfo node, Predicate<Long> condition,
}
}
+ /**
+ * Removes the node from the network topology when it becomes DEAD, and adds it back when it recovers.
+ * Doing it here, not in event handlers, keeps the topology in step with the node's health.
+ */
+ private void updateClusterMap(DatanodeInfo node, NodeStatus oldStatus, NodeStatus newStatus) {
+ try {
+ if (newStatus.isDead()) {
+ // Look up by path, as a re-registered node's DatanodeInfo may have no parent.
+ if (clusterMap.getNode(node.getNetworkFullPath()) != null) {
+ clusterMap.remove(node);
+ }
+ } else if (oldStatus.isDead()) {
+ clusterMap.add(node);
+ }
+ } catch (RuntimeException e) {
+ // Don't let this fail the health check.
+ LOG.error("Failed to update network topology for node {} moving from {} to {}", node,
+ oldStatus.getHealth(), newStatus.getHealth(), e);
+ }
+ }
+
private void fireHealthStateEvent(HddsProtos.NodeState health,
DatanodeDetails node) {
Event<DatanodeDetails> event = state2EventMap.get(health);
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 c8b100bd9c1..400b01ebdef 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
@@ -195,7 +195,7 @@ public SCMNodeManager(
HDDSLayoutVersionManager layoutVersionManager,
Function<String, String> nodeResolver) {
this.scmNodeEventPublisher = eventPublisher;
- this.nodeStateManager = new NodeStateManager(conf, eventPublisher,
+ this.nodeStateManager = new NodeStateManager(conf, eventPublisher, networkTopology,
layoutVersionManager, scmContext);
this.version = VersionInfo.getLatestVersion();
this.commandQueue = new CommandQueue();
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java
index 7b28de473ea..af006c7a41e 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDeadNodeHandler.java
@@ -21,12 +21,7 @@
import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType.RATIS;
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_DATANODE_RATIS_VOLUME_FREE_SPACE_MIN;
import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.clearInvocations;
-import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -36,9 +31,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Set;
-import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
import org.apache.hadoop.hdds.HddsConfigKeys;
import org.apache.hadoop.hdds.client.RatisReplicationConfig;
@@ -88,7 +81,6 @@ public class TestDeadNodeHandler {
private ContainerManager containerManager;
private PipelineManagerImpl pipelineManager;
private DeadNodeHandler deadNodeHandler;
- private HealthyReadOnlyNodeHandler healthyReadOnlyNodeHandler;
private EventPublisher publisher;
@TempDir
private File storageDir;
@@ -128,9 +120,6 @@ public void setup() throws IOException, AuthenticationException {
deletedBlockLog = mock(DeletedBlockLog.class);
deadNodeHandler = new DeadNodeHandler(nodeManager,
mock(PipelineManager.class), containerManager, deletedBlockLog);
- healthyReadOnlyNodeHandler =
- new HealthyReadOnlyNodeHandler(nodeManager,
- pipelineManager);
eventQueue.addHandler(SCMEvents.DEAD_NODE, deadNodeHandler);
publisher = mock(EventPublisher.class);
}
@@ -232,16 +221,10 @@ public void testOnMessage(@TempDir File tempDir) throws Exception {
// First set the node to IN_MAINTENANCE and ensure the container replicas
// are not removed on the dead event
datanode1 = nodeManager.getNode(datanode1.getID());
- assertTrue(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode1));
nodeManager.setNodeOperationalState(datanode1,
HddsProtos.NodeOperationalState.IN_MAINTENANCE);
setNodeHealthState(datanode1, HddsProtos.NodeState.DEAD);
deadNodeHandler.onMessage(datanode1, publisher);
- // make sure the node is removed from
- // ClusterNetworkTopology when it is considered as dead
- assertFalse(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode1));
verify(publisher, times(0)).fireEvent(SCMEvents.REPLICATION_MANAGER_NOTIFY, datanode1);
@@ -270,17 +253,11 @@ public void testOnMessage(@TempDir File tempDir) throws Exception {
nodeManager.setNodeOperationalState(datanode1,
HddsProtos.NodeOperationalState.IN_SERVICE);
// Changing the operational state of a DEAD node fires a DEAD_NODE event on
- // SCM's event queue. Let SCM's own DeadNodeHandler process it here, so its
- // asynchronous topology removal does not race with the handlers driven
- // below (it could otherwise remove the node right after
- // HealthyReadOnlyNodeHandler re-adds it).
+ // SCM's event queue. Let SCM's own DeadNodeHandler process it here, so it
+ // does not run concurrently with the handler driven below.
((EventQueue) scm.getEventQueue()).processAll(60000L);
setNodeHealthState(datanode1, HddsProtos.NodeState.DEAD);
deadNodeHandler.onMessage(datanode1, publisher);
- //datanode1 has been removed from ClusterNetworkTopology, another
- //deadNodeHandler.onMessage call will not change this
- assertFalse(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode1));
assertEquals(0, nodeManager.getCommandQueueCount(datanode1.getID(), cmd.getType()));
verify(publisher).fireEvent(SCMEvents.REPLICATION_MANAGER_NOTIFY, datanode1);
@@ -300,20 +277,14 @@ public void testOnMessage(@TempDir File tempDir) throws Exception {
.getContainerReplicas(ContainerID.valueOf(container3.getContainerID()));
assertEquals(1, container3Replicas.size());
assertEquals(datanode3, container3Replicas.iterator().next().getDatanodeDetails());
-
- //datanode will be added back to ClusterNetworkTopology if it resurrects
- healthyReadOnlyNodeHandler.onMessage(datanode1, publisher);
- assertTrue(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode1));
-
}
/**
- * Verifies that DeadNodeHandler skips topology removal when the node has
- * already been resurrected (state changed from DEAD to HEALTHY_READONLY).
+ * Verifies that DeadNodeHandler skips the event when the node is no longer DEAD, for example because it
+ * heartbeated again and became HEALTHY_READONLY before the event was handled.
*/
@Test
- public void testDeadNodeHandlerSkipsRemovalWhenNodeResurrected(
+ public void testDeadNodeHandlerSkipsNodeThatIsNoLongerDead(
@TempDir File tempDir) throws Exception {
DatanodeDetails datanode = MockDatanodeDetails.randomDatanodeDetails();
String storagePath = tempDir.getPath()
@@ -332,147 +303,13 @@ public void testDeadNodeHandlerSkipsRemovalWhenNodeResurrected(
Arrays.asList(metaStorageReport)), null);
datanode = nodeManager.getNode(datanode.getID());
- assertTrue(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode));
-
- // Simulate: DEAD_NODE event was fired, but before DeadNodeHandler
- // processes it, the node heartbeated and was resurrected to
- // HEALTHY_READONLY. The handler should see the current state and
- // skip removal.
+ // The DEAD_NODE event was fired, but the node heartbeated again and became HEALTHY_READONLY before
+ // DeadNodeHandler processed it. The handler should see the current state and skip the event.
setNodeHealthState(datanode, HddsProtos.NodeState.HEALTHY_READONLY);
deadNodeHandler.onMessage(datanode, publisher);
- assertTrue(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode),
- "Node should remain in topology when it has been resurrected");
- }
-
- /**
- * Verifies that HealthyReadOnlyNodeHandler re-adds a node to topology
- * even if it was removed by a concurrent DeadNodeHandler.
- */
- @Test
- public void testHealthyReadOnlyHandlerAddsRemovedNode(
- @TempDir File tempDir) throws Exception {
- DatanodeDetails datanode = MockDatanodeDetails.randomDatanodeDetails();
- String storagePath = tempDir.getPath()
- .concat("/data-" + datanode.getID());
- String metaStoragePath = tempDir.getPath()
- .concat("/metadata-" + datanode.getID());
- StorageReportProto storageReport = HddsTestUtils.createStorageReport(
- datanode.getID(), storagePath, 100 * OzoneConsts.TB,
- 10 * OzoneConsts.TB, 90 * OzoneConsts.TB, null);
- MetadataStorageReportProto metaStorageReport =
- HddsTestUtils.createMetadataStorageReport(metaStoragePath,
- 100 * OzoneConsts.GB, 10 * OzoneConsts.GB,
- 90 * OzoneConsts.GB, null);
- nodeManager.register(datanode,
- HddsTestUtils.createNodeReport(Arrays.asList(storageReport),
- Arrays.asList(metaStorageReport)), null);
- datanode = nodeManager.getNode(datanode.getID());
-
- // Manually remove node from topology to simulate DeadNodeHandler
- // having run first.
- nodeManager.getClusterNetworkTopologyMap().remove(datanode);
- assertFalse(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode));
-
- // HealthyReadOnlyNodeHandler should add it back unconditionally.
- healthyReadOnlyNodeHandler.onMessage(datanode, publisher);
- assertTrue(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode),
- "Node should be re-added to topology by HealthyReadOnlyNodeHandler");
- }
-
- /**
- * Reproduces the race condition between DeadNodeHandler and
- * HealthyReadOnlyNodeHandler where interleaved execution could leave
- * a resurrected node missing from the network topology.
- *
- * The interleaving being tested:
- * 1. DeadNodeHandler starts processing (slow: closing containers, etc.)
- * 2. Node is resurrected (DEAD -> HEALTHY_READONLY) via heartbeat
- * 3. HealthyReadOnlyNodeHandler runs, sees node in topology, does not add
- * 4. DeadNodeHandler finishes and removes node from topology
- *
- * With the fix: step 4 checks current state and skips removal.
- */
- @Test
- public void testDeadNodeAndHealthyReadOnlyRaceCondition(
- @TempDir File tempDir) throws Exception {
- DatanodeDetails datanode = MockDatanodeDetails.randomDatanodeDetails();
- String storagePath = tempDir.getPath()
- .concat("/data-" + datanode.getID());
- String metaStoragePath = tempDir.getPath()
- .concat("/metadata-" + datanode.getID());
- StorageReportProto storageReport = HddsTestUtils.createStorageReport(
- datanode.getID(), storagePath, 100 * OzoneConsts.TB,
- 10 * OzoneConsts.TB, 90 * OzoneConsts.TB, null);
- MetadataStorageReportProto metaStorageReport =
- HddsTestUtils.createMetadataStorageReport(metaStoragePath,
- 100 * OzoneConsts.GB, 10 * OzoneConsts.GB,
- 90 * OzoneConsts.GB, null);
- nodeManager.register(datanode,
- HddsTestUtils.createNodeReport(Arrays.asList(storageReport),
- Arrays.asList(metaStorageReport)), null);
- datanode = nodeManager.getNode(datanode.getID());
-
- assertTrue(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode));
-
- // Block DeadNodeHandler just before the topology removal by making
- // deletedBlockLog.onDatanodeDead() pause. This call happens right
- // before the topology check in the handler.
- CountDownLatch deadHandlerBlocked = new CountDownLatch(1);
- CountDownLatch proceedWithRemoval = new CountDownLatch(1);
-
- DeletedBlockLog blockingDeletedBlockLog = mock(DeletedBlockLog.class);
- doAnswer(invocation -> {
- deadHandlerBlocked.countDown();
- proceedWithRemoval.await();
- return null;
- }).when(blockingDeletedBlockLog).onDatanodeDead(any());
-
- setNodeHealthState(datanode, HddsProtos.NodeState.DEAD);
-
- DeadNodeHandler blockingDeadHandler = new DeadNodeHandler(nodeManager,
- mock(PipelineManager.class), containerManager,
- blockingDeletedBlockLog);
-
- DatanodeDetails finalDatanode = datanode;
- AtomicReference<Exception> threadException = new AtomicReference<>();
- Thread deadHandlerThread = new Thread(() -> {
- try {
- blockingDeadHandler.onMessage(finalDatanode, publisher);
- } catch (Exception e) {
- threadException.set(e);
- }
- });
- deadHandlerThread.start();
-
- // Wait for DeadNodeHandler to be blocked mid-execution.
- assertTrue(deadHandlerBlocked.await(10, TimeUnit.SECONDS),
- "DeadNodeHandler should have started processing");
-
- // Simulate resurrection: node transitions to HEALTHY_READONLY.
- setNodeHealthState(datanode, HddsProtos.NodeState.HEALTHY_READONLY);
-
- // HealthyReadOnlyNodeHandler runs while DeadNodeHandler is blocked.
- healthyReadOnlyNodeHandler.onMessage(datanode, publisher);
-
- // Release the DeadNodeHandler to finish.
- proceedWithRemoval.countDown();
- deadHandlerThread.join(10_000);
-
- assertNull(threadException.get(),
- "DeadNodeHandler should not throw");
-
- // With the fix, the node should remain in topology because
- // DeadNodeHandler sees the node is no longer DEAD and skips removal.
- assertTrue(
- nodeManager.getClusterNetworkTopologyMap().contains(datanode),
- "Resurrected node must remain in topology after race between "
- + "DeadNodeHandler and HealthyReadOnlyNodeHandler");
+ verify(publisher, times(0)).fireEvent(SCMEvents.REPLICATION_MANAGER_NOTIFY, datanode);
+ verify(deletedBlockLog, times(0)).onDatanodeDead(datanode.getID());
}
private void setNodeHealthState(DatanodeDetails datanode,
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeStateManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeStateManager.java
index d10d951da90..1280ba045e6 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeStateManager.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeStateManager.java
@@ -21,7 +21,10 @@
import static org.apache.hadoop.ozone.container.upgrade.UpgradeUtils.defaultLayoutVersionProto;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -32,6 +35,7 @@
import java.util.Set;
import java.util.UUID;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState;
@@ -39,6 +43,8 @@
import org.apache.hadoop.hdds.scm.container.ContainerID;
import org.apache.hadoop.hdds.scm.events.SCMEvents;
import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.net.NetworkTopology;
+import org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl;
import org.apache.hadoop.hdds.scm.node.states.NodeAlreadyExistsException;
import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
import org.apache.hadoop.hdds.scm.server.upgrade.FinalizationCheckpoint;
@@ -67,6 +73,8 @@ public class TestNodeStateManager {
private ConfigurationSource conf;
private MockEventPublisher eventPublisher;
private SCMContext scmContext;
+ private NetworkTopology clusterMap;
+ private LayoutVersionManager mockVersionManager;
private int scmSlv;
private int scmMlv;
@@ -96,11 +104,11 @@ public char[] getPassword(String key) throws IOException {
eventPublisher = new MockEventPublisher();
scmSlv = maxLayoutVersion();
scmMlv = maxLayoutVersion();
- LayoutVersionManager mockVersionManager = mock(HDDSLayoutVersionManager.class);
+ mockVersionManager = mock(HDDSLayoutVersionManager.class);
when(mockVersionManager.getMetadataLayoutVersion()).thenReturn(scmMlv);
when(mockVersionManager.getSoftwareLayoutVersion()).thenReturn(scmSlv);
- nsm = new NodeStateManager(conf, eventPublisher, mockVersionManager,
- scmContext);
+ clusterMap = new NetworkTopologyImpl(new OzoneConfiguration());
+ nsm = new NodeStateManager(conf, eventPublisher, clusterMap, mockVersionManager, scmContext);
}
@Test
@@ -254,6 +262,77 @@ public void testNodeCanTransitionThroughHealthStatesAndFiresEvents()
}
}
+ @Test
+ public void testDeadNodeIsRemovedFromTopologyUntilItRecovers()
+ throws NodeAlreadyExistsException, NodeNotFoundException {
+ long now = Time.monotonicNow();
+ long staleLimit = HddsServerUtil.getStaleNodeInterval(conf) + 1000;
+ long deadLimit = HddsServerUtil.getDeadNodeInterval(conf) + 1000;
+
+ // Like SCMNodeManager, add the node to the topology before adding it to NodeStateManager.
+ DatanodeDetails dn = generateDatanode();
+ dn.setNetworkName(dn.getUuidString());
+ clusterMap.add(dn);
+ nsm.addNode(dn, defaultLayoutVersionProto());
+ DatanodeInfo dni = nsm.getNode(dn);
+ String path = dn.getNetworkFullPath();
+
+ dni.updateLastHeartbeatTime(now - staleLimit);
+ nsm.checkNodesHealth();
+ assertEquals(NodeState.STALE, nsm.getNodeStatus(dn).getHealth());
+ assertNotNull(clusterMap.getNode(path));
+
+ dni.updateLastHeartbeatTime(now - deadLimit);
+ nsm.checkNodesHealth();
+ assertEquals(NodeState.DEAD, nsm.getNodeStatus(dn).getHealth());
+ assertNull(clusterMap.getNode(path));
+
+ dni.updateLastHeartbeatTime();
+ nsm.checkNodesHealth();
+ assertEquals(NodeState.HEALTHY_READONLY, nsm.getNodeStatus(dn).getHealth());
+ assertNotNull(clusterMap.getNode(path));
+ }
+
+ @Test
+ public void testReregisteredNodeIsRemovedFromTopologyWhenDead()
+ throws NodeAlreadyExistsException, NodeNotFoundException {
+ DatanodeDetails dn = generateDatanode();
+ dn.setNetworkName(dn.getUuidString());
+ clusterMap.add(dn);
+ nsm.addNode(dn, defaultLayoutVersionProto());
+ String path = dn.getNetworkFullPath();
+
+ // Re-registering with a new version or new ports replaces the DatanodeInfo with a copy that has no parent.
+ DatanodeDetails reregistered = new DatanodeDetails(dn);
+ reregistered.setParent(null);
+ nsm.updateNode(reregistered, defaultLayoutVersionProto());
+
+ nsm.getNode(dn).updateLastHeartbeatTime(Time.monotonicNow() - HddsServerUtil.getDeadNodeInterval(conf) - 1000);
+ nsm.checkNodesHealth();
+ nsm.checkNodesHealth();
+
+ assertEquals(NodeState.DEAD, nsm.getNodeStatus(dn).getHealth());
+ assertNull(clusterMap.getNode(path));
+ }
+
+ @Test
+ public void testTopologyUpdateFailureDoesNotFailHealthCheck()
+ throws NodeAlreadyExistsException, NodeNotFoundException {
+ NetworkTopology failingClusterMap = mock(NetworkTopology.class);
+ when(failingClusterMap.contains(any())).thenReturn(true);
+ doThrow(new IllegalStateException("injected")).when(failingClusterMap).remove(any());
+ nsm = new NodeStateManager(conf, eventPublisher, failingClusterMap, mockVersionManager, scmContext);
+
+ DatanodeDetails dn = generateDatanode();
+ nsm.addNode(dn, defaultLayoutVersionProto());
+ nsm.getNode(dn).updateLastHeartbeatTime(Time.monotonicNow() - HddsServerUtil.getDeadNodeInterval(conf) - 1000);
+ nsm.checkNodesHealth();
+ nsm.checkNodesHealth();
+
+ assertEquals(NodeState.DEAD, nsm.getNodeStatus(dn).getHealth());
+ assertEquals(SCMEvents.DEAD_NODE, eventPublisher.getLastEvent());
+ }
+
@Test
public void testNodeOpStateCanBeSet()
throws NodeAlreadyExistsException, NodeNotFoundException {
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestQueryNode.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestQueryNode.java
index f707580a246..d4de3901a37 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestQueryNode.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestQueryNode.java
@@ -27,14 +27,18 @@
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DEADNODE_INTERVAL;
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_STALENODE_INTERVAL;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.io.IOException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient;
+import org.apache.hadoop.hdds.scm.net.NetworkTopology;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
import org.apache.hadoop.ozone.MiniOzoneCluster;
import org.apache.ozone.test.GenericTestUtils;
import org.junit.jupiter.api.AfterEach;
@@ -109,4 +113,23 @@ public void testStaleNodesCount() throws Exception {
HddsProtos.QueryScope.CLUSTER, "").size();
assertEquals(2, nodeCount, "Mismatch of expected nodes count");
}
+
+ /**
+ * A datanode that dies is removed from SCM's network topology, and added back once it is restarted.
+ */
+ @Test
+ public void testDeadNodeIsRemovedFromTopologyUntilRestarted() throws Exception {
+ StorageContainerManager scm = cluster.getStorageContainerManager();
+ NetworkTopology topology = scm.getClusterMap();
+ DatanodeDetails dn = cluster.getHddsDatanodes().get(0).getDatanodeDetails();
+ String path = scm.getScmNodeManager().getNode(dn.getID()).getNetworkFullPath();
+ assertNotNull(topology.getNode(path));
+
+ cluster.shutdownHddsDatanode(dn);
+ GenericTestUtils.waitFor(() -> topology.getNode(path) == null, 100, 30_000);
+ assertEquals(DEAD, scm.getScmNodeManager().getNodeStatus(dn).getHealth());
+
+ cluster.restartHddsDatanode(dn, true);
+ assertNotNull(topology.getNode(path));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org