diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java index 6e758d6ebcb..4399466b0eb 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java @@ -1747,24 +1747,16 @@ public void update(LiveTServerSet current, Set deleted, serversToShutdown.retainAll(current.getCurrentServers()); } - private static void cleanListByHostAndPort(Collection badServers, + static void cleanListByHostAndPort(Collection badServers, Set deleted, Set added) { - Iterator badIter = badServers.iterator(); - while (badIter.hasNext()) { - TServerInstance bad = badIter.next(); - for (TServerInstance add : added) { - if (bad.getHostPort().equals(add.getHostPort())) { - badIter.remove(); - break; - } - } - for (TServerInstance del : deleted) { - if (bad.getHostPort().equals(del.getHostPort())) { - badIter.remove(); - break; - } - } + if (badServers.isEmpty() || (deleted.isEmpty() && added.isEmpty())) { + // nothing to do + return; } + HashSet removalSet = new HashSet<>(deleted.size() + added.size()); + deleted.forEach(tsi -> removalSet.add(tsi.getHostAndPort())); + added.forEach(tsi -> removalSet.add(tsi.getHostAndPort())); + badServers.removeIf(badServer -> removalSet.contains(badServer.getHostAndPort())); } @Override diff --git a/server/manager/src/test/java/org/apache/accumulo/manager/ManagerTest.java b/server/manager/src/test/java/org/apache/accumulo/manager/ManagerTest.java new file mode 100644 index 00000000000..4bd9ad3386f --- /dev/null +++ b/server/manager/src/test/java/org/apache/accumulo/manager/ManagerTest.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.accumulo.manager; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.apache.accumulo.core.metadata.TServerInstance; +import org.junit.jupiter.api.Test; + +public class ManagerTest { + + @Test + public void cleanByHostTest() { + final TServerInstance server1 = new TServerInstance("host1:1234[SESSION1]"); + final TServerInstance server2 = new TServerInstance("host1:1234[SESSION2]"); + final TServerInstance server3 = new TServerInstance("host1:1235[SESSION3]"); + final TServerInstance server4 = new TServerInstance("host2:1234[SESSION4]"); + + Set servers = new HashSet<>(List.of(server1, server2, server3, server4)); + // check deleted set removes by host port + Manager.cleanListByHostAndPort(servers, Set.of(new TServerInstance("host1:1234[SESSION5]")), + Set.of()); + assertEquals(Set.of(server3, server4), servers); + + servers = new HashSet<>(List.of(server1, server2, server3, server4)); + // check added set removes by host port + Manager.cleanListByHostAndPort(servers, Set.of(), + Set.of(new TServerInstance("host1:1234[SESSION5]"))); + assertEquals(Set.of(server3, server4), servers); + + // check using both sets + servers = new HashSet<>(List.of(server1, server2, server3, server4)); + Manager.cleanListByHostAndPort(servers, Set.of(new TServerInstance("host1:1235[SESSION5]")), + Set.of(new TServerInstance("host1:1234[SESSION6]"))); + assertEquals(Set.of(server4), servers); + + // Test empty sets + servers = new HashSet<>(List.of(server1, server2, server3, server4)); + Manager.cleanListByHostAndPort(servers, Set.of(), Set.of()); + assertEquals(Set.of(server1, server2, server3, server4), servers); + servers.clear(); + Manager.cleanListByHostAndPort(servers, Set.of(), + Set.of(new TServerInstance("host1:1234[SESSION5]"))); + assertEquals(Set.of(), servers); + } +}