[grid] No need to register a Node that is DOWN

Fixes SeleniumHQ/docker-selenium#1640
SeleniumHQ · Aug 3, 2022 · a9a526e · a9a526e
1 parent 7ce42b7
commit a9a526e
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 5 deletions.
diff --git a/java/src/org/openqa/selenium/grid/distributor/GridModel.java b/java/src/org/openqa/selenium/grid/distributor/GridModel.java
@@ -210,6 +210,8 @@ public void purgeDeadNodes() {
       for (NodeStatus node : nodes) {
         NodeId id = node.getNodeId();
         if (nodeHealthCount.getOrDefault(id, 0) > UNHEALTHY_THRESHOLD) {
+          LOG.info(String.format("Removing Node %s, unhealthy threshold has been reached",
+                                 node.getExternalUri()));
           toRemove.add(node);
           break;
         }

diff --git a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java
@@ -285,6 +285,13 @@ private void register(NodeStatus status) {
         return;
       }
 
+      if (status.getAvailability() != UP) {
+        // A Node might be draining or down (in the case of Relay nodes)
+        // but the heartbeat is still running.
+        // We do not need to add this Node for now.
+        return;
+      }
+
       Set<Capabilities> capabilities = status.getSlots().stream()
         .map(Slot::getStereotype)
         .map(ImmutableCapabilities::copyOf)
@@ -314,9 +321,10 @@ public LocalDistributor add(Node node) {
     NodeStatus initialNodeStatus;
     try {
       initialNodeStatus = node.getStatus();
-      if (initialNodeStatus.getAvailability() == DRAINING) {
-        // A Node might be draining but the heartbeat is still running.
-        // We do not need to add this Node again.
+      if (initialNodeStatus.getAvailability() != UP) {
+        // A Node might be draining or down (in the case of Relay nodes)
+        // but the heartbeat is still running.
+        // We do not need to add this Node for now.
         return this;
       }
       model.add(initialNodeStatus);
@@ -383,13 +391,13 @@ private Runnable asRunnableHealthCheck(Node node) {
     return () -> {
       boolean checkFailed = false;
       Exception failedCheckException = null;
-      LOG.log(getDebugLogLevel(), "Running health check for " + node.getUri());
+      LOG.log(getDebugLogLevel(), "Running healthcheck for Node " + node.getUri());
 
       HealthCheck.Result result;
       try {
         result = healthCheck.check();
       } catch (Exception e) {
-        LOG.log(Level.WARNING, "Unable to process node " + id, e);
+        LOG.log(Level.WARNING, "Unable to process Node healthcheck " + id, e);
         result = new HealthCheck.Result(DOWN, "Unable to run healthcheck. Assuming down");
         checkFailed = true;
         failedCheckException = e;