diff --git a/java/src/org/openqa/selenium/grid/distributor/GridModel.java b/java/src/org/openqa/selenium/grid/distributor/GridModel.java index c187de3a2ab4a..49892f2680a4c 100644 --- a/java/src/org/openqa/selenium/grid/distributor/GridModel.java +++ b/java/src/org/openqa/selenium/grid/distributor/GridModel.java @@ -114,7 +114,9 @@ public void add(NodeStatus node) { "Re-adding node with id %s and URI %s.", node.getNodeId(), node.getExternalUri())); - events.fire(new NodeRestartedEvent(node)); + // Send the previous state to allow cleaning up the old node related resources. + // Nodes are initially added in the "down" state, so the new state must be ignored. + events.fire(new NodeRestartedEvent(next)); iterator.remove(); break; } @@ -226,7 +228,8 @@ public void purgeDeadNodes() { if (nodeHealthCount.getOrDefault(id, 0) > UNHEALTHY_THRESHOLD) { LOG.info( String.format( - "Removing Node %s, unhealthy threshold has been reached", node.getExternalUri())); + "Removing Node %s (uri: %s), unhealthy threshold has been reached", + node.getNodeId(), node.getExternalUri())); toRemove.add(node); break; } @@ -239,11 +242,17 @@ public void purgeDeadNodes() { lastTouched.plus(node.getHeartbeatPeriod().multipliedBy(PURGE_TIMEOUT_MULTIPLIER)); if (node.getAvailability() == UP && lostTime.isBefore(now)) { - LOG.info(String.format("Switching Node %s from UP to DOWN", node.getExternalUri())); + LOG.info( + String.format( + "Switching Node %s (uri: %s) from UP to DOWN", + node.getNodeId(), node.getExternalUri())); replacements.put(node, rewrite(node, DOWN)); nodePurgeTimes.put(id, Instant.now()); } else if (node.getAvailability() == DOWN && deadTime.isBefore(now)) { - LOG.info(String.format("Removing Node %s, DOWN for too long", node.getExternalUri())); + LOG.info( + String.format( + "Removing Node %s (uri: %s), DOWN for too long", + node.getNodeId(), node.getExternalUri())); toRemove.add(node); } } diff --git a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java index b6db25c5dbfab..d6cf622d237ac 100644 --- a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java +++ b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java @@ -206,7 +206,8 @@ public LocalDistributor( bus.addListener(NodeStatusEvent.listener(this::register)); bus.addListener(NodeStatusEvent.listener(model::refresh)); - bus.addListener(NodeRestartedEvent.listener(this::handleNodeRestarted)); + bus.addListener( + NodeRestartedEvent.listener(previousNodeStatus -> remove(previousNodeStatus.getNodeId()))); bus.addListener(NodeRemovedEvent.listener(nodeStatus -> remove(nodeStatus.getNodeId()))); bus.addListener( NodeHeartBeatEvent.listener( @@ -329,25 +330,6 @@ private void register(NodeStatus status) { } } - private void handleNodeRestarted(NodeStatus status) { - Require.nonNull("Node", status); - Lock writeLock = lock.writeLock(); - writeLock.lock(); - try { - if (!nodes.containsKey(status.getNodeId())) { - return; - } - if (!getNodeFromURI(status.getExternalUri()).isDraining()) { - LOG.info( - String.format( - "Node %s has restarted. Setting availability to DOWN.", status.getNodeId())); - model.setAvailability(status.getNodeId(), DOWN); - } - } finally { - writeLock.unlock(); - } - } - @Override public LocalDistributor add(Node node) { Require.nonNull("Node", node); @@ -499,15 +481,13 @@ public void remove(NodeId nodeId) { Lock writeLock = lock.writeLock(); writeLock.lock(); try { - Node node = nodes.get(nodeId); + Node node = nodes.remove(nodeId); + model.remove(nodeId); + allChecks.remove(nodeId); if (node instanceof RemoteNode) { ((RemoteNode) node).close(); } - - nodes.remove(nodeId); - model.remove(nodeId); - allChecks.remove(nodeId); } finally { writeLock.unlock(); } diff --git a/java/src/org/openqa/selenium/grid/sessionmap/jdbc/JdbcBackedSessionMap.java b/java/src/org/openqa/selenium/grid/sessionmap/jdbc/JdbcBackedSessionMap.java index aeab49abd3f13..eb31465a1fbca 100644 --- a/java/src/org/openqa/selenium/grid/sessionmap/jdbc/JdbcBackedSessionMap.java +++ b/java/src/org/openqa/selenium/grid/sessionmap/jdbc/JdbcBackedSessionMap.java @@ -92,7 +92,8 @@ public JdbcBackedSessionMap(Tracer tracer, Connection jdbcConnection, EventBus b .forEach(this::remove))); bus.addListener( - NodeRestartedEvent.listener(nodeStatus -> this.removeByUri(nodeStatus.getExternalUri()))); + NodeRestartedEvent.listener( + previousNodeStatus -> this.removeByUri(previousNodeStatus.getExternalUri()))); } public static SessionMap create(Config config) { diff --git a/java/src/org/openqa/selenium/grid/sessionmap/local/LocalSessionMap.java b/java/src/org/openqa/selenium/grid/sessionmap/local/LocalSessionMap.java index 6e2ff03859170..df62989b5bb4e 100644 --- a/java/src/org/openqa/selenium/grid/sessionmap/local/LocalSessionMap.java +++ b/java/src/org/openqa/selenium/grid/sessionmap/local/LocalSessionMap.java @@ -67,10 +67,11 @@ public LocalSessionMap(Tracer tracer, EventBus bus) { bus.addListener( NodeRestartedEvent.listener( - nodeStatus -> { + previousNodeStatus -> { List toRemove = knownSessions.entrySet().stream() - .filter((e) -> e.getValue().getUri().equals(nodeStatus.getExternalUri())) + .filter( + (e) -> e.getValue().getUri().equals(previousNodeStatus.getExternalUri())) .map(Map.Entry::getKey) .collect(Collectors.toList()); diff --git a/java/src/org/openqa/selenium/grid/sessionmap/redis/RedisBackedSessionMap.java b/java/src/org/openqa/selenium/grid/sessionmap/redis/RedisBackedSessionMap.java index 3e3278c4fcfec..62b1220edb21a 100644 --- a/java/src/org/openqa/selenium/grid/sessionmap/redis/RedisBackedSessionMap.java +++ b/java/src/org/openqa/selenium/grid/sessionmap/redis/RedisBackedSessionMap.java @@ -87,7 +87,8 @@ public RedisBackedSessionMap(Tracer tracer, URI serverUri, EventBus bus) { .forEach(this::remove))); bus.addListener( - NodeRestartedEvent.listener(nodeStatus -> this.removeByUri(nodeStatus.getExternalUri()))); + NodeRestartedEvent.listener( + previousNodeStatus -> this.removeByUri(previousNodeStatus.getExternalUri()))); } public static SessionMap create(Config config) {