Cleanup HostRegistry, no functional changes

6 files changed, 49 insertions, 62 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
index 5831cb3e75f..19f41b514ec 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
@@ -253,7 +253,7 @@ public class TenantApplications implements RequestHandler, HostValidator {
 
         if (hasApplication(applicationId)) {
             applicationMapper.remove(applicationId);
-            hostRegistry.removeHostsForKey(applicationId);
+            hostRegistry.removeHosts(applicationId);
             configActivationListenersOnRemove(applicationId);
             tenantMetricUpdater.setApplications(applicationMapper.numApplications());
             metrics.removeMetricUpdater(Metrics.createDimensions(applicationId));
@@ -277,17 +277,16 @@ public class TenantApplications implements RequestHandler, HostValidator {
     }
 
     private void configActivationListenersOnRemove(ApplicationId applicationId) {
-        configActivationListener.hostsUpdated(applicationId, hostRegistry.getHostsForKey(applicationId));
+        configActivationListener.hostsUpdated(applicationId, hostRegistry.getHosts(applicationId));
         configActivationListener.applicationRemoved(applicationId);
     }
 
     private void setActiveApp(ApplicationSet applicationSet) {
-        ApplicationId id = applicationSet.getId();
-        Collection<String> hostsForApp = applicationSet.getAllHosts();
-        hostRegistry.update(id, hostsForApp);
+        ApplicationId applicationId = applicationSet.getId();
+        hostRegistry.update(applicationId, applicationSet.getAllHosts());
         applicationSet.updateHostMetrics();
         tenantMetricUpdater.setApplications(applicationMapper.numApplications());
-        applicationMapper.register(id, applicationSet);
+        applicationMapper.register(applicationId, applicationSet);
     }
 
     @Override
@@ -377,7 +376,7 @@ public class TenantApplications implements RequestHandler, HostValidator {
 
     @Override
     public ApplicationId resolveApplicationId(String hostName) {
-        return hostRegistry.getKeyForHost(hostName);
+        return hostRegistry.getApplicationId(hostName);
     }
 
     @Override
@@ -403,7 +402,7 @@ public class TenantApplications implements RequestHandler, HostValidator {
     }
 
     public ApplicationId getApplicationIdForHostName(String hostname) {
-        return hostRegistry.getKeyForHost(hostname);
+        return hostRegistry.getApplicationId(hostname);
     }
 
     public TenantFileSystemDirs getTenantFileSystemDirs() { return tenantFileSystemDirs; }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java b/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java
index b89f3bba835..1a7408d6251 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java
@@ -9,35 +9,26 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 /**
- * A host registry with a mapping between hosts (hostname as a String) and some type T
- * TODO: Maybe we should have a Host type, but using String for now.
+ * A host registry with a mapping between hostname and ApplicationId
  *
  * @author Ulf Lilleengen
  */
 public class HostRegistry implements HostValidator {
 
-    private static final Logger log = Logger.getLogger(HostRegistry.class.getName());
+    private final Map<String, ApplicationId> host2ApplicationId = new ConcurrentHashMap<>();
 
-    private final Map<String, ApplicationId> host2KeyMap = new ConcurrentHashMap<>();
-
-    public ApplicationId getKeyForHost(String hostName) {
-        return host2KeyMap.get(hostName);
+    public ApplicationId getApplicationId(String hostName) {
+        return host2ApplicationId.get(hostName);
     }
 
-    public synchronized void update(ApplicationId key, Collection<String> newHosts) {
-        verifyHosts(key, newHosts);
-        Collection<String> currentHosts = getHostsForKey(key);
-        log.log(Level.FINE, () -> "Setting hosts for key '" + key + "', " +
-                                  "newHosts: " + newHosts + ", " +
-                                  "currentHosts: " + currentHosts);
-        Collection<String> removedHosts = getRemovedHosts(newHosts, currentHosts);
+    public synchronized void update(ApplicationId applicationId, Collection<String> newHosts) {
+        verifyHosts(applicationId, newHosts);
+        Collection<String> removedHosts = findRemovedHosts(newHosts, getHosts(applicationId));
         removeHosts(removedHosts);
-        addHosts(key, newHosts);
+        addHosts(applicationId, newHosts);
     }
 
     @Override
@@ -45,49 +36,47 @@ public class HostRegistry implements HostValidator {
         for (String host : newHosts) {
             if (hostAlreadyTaken(host, applicationId)) {
                 throw new IllegalArgumentException("'" + applicationId + "' tried to allocate host '" + host +
-                                                   "', but the host is already taken by '" + host2KeyMap.get(host) + "'");
+                                                   "', but the host is already taken by '" + host2ApplicationId.get(host) + "'");
             }
         }
     }
 
-    public synchronized void removeHostsForKey(ApplicationId key) {
-        host2KeyMap.entrySet().removeIf(entry -> entry.getValue().equals(key));
+    public synchronized void removeHosts(ApplicationId applicationId) {
+        host2ApplicationId.entrySet().removeIf(entry -> entry.getValue().equals(applicationId));
     }
 
-    public synchronized void removeHostsForKey(TenantName key) {
-        host2KeyMap.entrySet().removeIf(entry -> entry.getValue().tenant().equals(key));
+    public synchronized void removeHosts(TenantName tenantName) {
+        host2ApplicationId.entrySet().removeIf(entry -> entry.getValue().tenant().equals(tenantName));
     }
 
     public synchronized Collection<String> getAllHosts() {
-        return Collections.unmodifiableCollection(new ArrayList<>(host2KeyMap.keySet()));
+        return Collections.unmodifiableCollection(new ArrayList<>(host2ApplicationId.keySet()));
     }
 
-    public synchronized Collection<String> getHostsForKey(ApplicationId key) {
-        return host2KeyMap.entrySet().stream()
-                .filter(entry -> entry.getValue().equals(key))
-                .map(Map.Entry::getKey)
-                .collect(Collectors.toSet());
+    public synchronized Collection<String> getHosts(ApplicationId applicationId) {
+        return host2ApplicationId.entrySet().stream()
+                                 .filter(entry -> entry.getValue().equals(applicationId))
+                                 .map(Map.Entry::getKey)
+                                 .collect(Collectors.toSet());
     }
 
-    private boolean hostAlreadyTaken(String host, ApplicationId key) {
-        return host2KeyMap.containsKey(host) && !key.equals(host2KeyMap.get(host));
+    private boolean hostAlreadyTaken(String host, ApplicationId applicationId) {
+        return host2ApplicationId.containsKey(host) && !applicationId.equals(host2ApplicationId.get(host));
     }
 
-    private static Collection<String> getRemovedHosts(Collection<String> newHosts, Collection<String> previousHosts) {
+    private static Collection<String> findRemovedHosts(Collection<String> newHosts, Collection<String> previousHosts) {
         return Collections2.filter(previousHosts, host -> !newHosts.contains(host));
     }
 
-    private void removeHosts(Collection<String> removedHosts) {
-        for (String host : removedHosts) {
-            log.log(Level.FINE, () -> "Removing " + host);
-            host2KeyMap.remove(host);
+    private void removeHosts(Collection<String> hosts) {
+        for (String host : hosts) {
+            host2ApplicationId.remove(host);
         }
     }
 
-    private void addHosts(ApplicationId key, Collection<String> newHosts) {
-        for (String host : newHosts) {
-            log.log(Level.FINE, () -> "Adding " + host);
-            host2KeyMap.put(host, key);
+    private void addHosts(ApplicationId key, Collection<String> hosts) {
+        for (String host : hosts) {
+            host2ApplicationId.put(host, key);
         }
     }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
index be4738258d8..aac34238b90 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java
@@ -351,7 +351,7 @@ public class RpcServer implements Runnable, ConfigActivationListener, TenantList
     Optional<TenantName> resolveTenant(JRTServerConfigRequest request, Trace trace) {
         if ("*".equals(request.getConfigKey().getConfigId())) return Optional.of(ApplicationId.global().tenant());
         String hostname = request.getClientHostName();
-        ApplicationId applicationId = hostRegistry.getKeyForHost(hostname);
+        ApplicationId applicationId = hostRegistry.getApplicationId(hostname);
         if (applicationId == null) {
             if (GetConfigProcessor.logDebug(trace)) {
                 String message = "Did not find tenant for host '" + hostname + "', using " + TenantName.defaultName() +
@@ -445,7 +445,7 @@ public class RpcServer implements Runnable, ConfigActivationListener, TenantList
         log.log(Level.FINE, () -> TenantRepository.logPre(tenant) +
                             "Tenant deleted, removing request handler and cleaning host registry");
         tenants.remove(tenant);
-        hostRegistry.removeHostsForKey(tenant);
+        hostRegistry.removeHosts(tenant);
     }
 
     @Override
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java
index 536a446df2f..21f7354401f 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java
@@ -106,7 +106,7 @@ public class MultiTenantRpcAuthorizer implements RpcAuthorizer {
                     return; // global config access ok
                 } else {
                     String hostname = configRequest.getClientHostName();
-                    ApplicationId applicationId = hostRegistry.getKeyForHost(hostname);
+                    ApplicationId applicationId = hostRegistry.getApplicationId(hostname);
                     if (applicationId == null) {
                         if (isConfigKeyForSentinelConfig(configKey)) {
                             return; // config processor will return empty sentinel config for unknown nodes
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java
index df00d28134f..646017a498e 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java
@@ -24,23 +24,23 @@ public class HostRegistryTest {
     @Test
     public void old_hosts_are_removed() {
         HostRegistry reg = new HostRegistry();
-        assertNull(reg.getKeyForHost("foo.com"));
+        assertNull(reg.getApplicationId("foo.com"));
         reg.update(foo, List.of("foo.com", "bar.com", "baz.com"));
         assertGetKey(reg, "foo.com", foo);
         assertGetKey(reg, "bar.com", foo);
         assertGetKey(reg, "baz.com", foo);
         assertEquals(3, reg.getAllHosts().size());
         reg.update(foo, List.of("bar.com", "baz.com"));
-        assertNull(reg.getKeyForHost("foo.com"));
+        assertNull(reg.getApplicationId("foo.com"));
         assertGetKey(reg, "bar.com", foo);
         assertGetKey(reg, "baz.com", foo);
 
         assertEquals(2, reg.getAllHosts().size());
         assertTrue(reg.getAllHosts().containsAll(List.of("bar.com", "baz.com")));
-        reg.removeHostsForKey(foo);
+        reg.removeHosts(foo);
         assertTrue(reg.getAllHosts().isEmpty());
-        assertNull(reg.getKeyForHost("foo.com"));
-        assertNull(reg.getKeyForHost("bar.com"));
+        assertNull(reg.getApplicationId("foo.com"));
+        assertNull(reg.getApplicationId("bar.com"));
     }
 
     @Test
@@ -74,9 +74,9 @@ public class HostRegistryTest {
         HostRegistry reg = new HostRegistry();
         List<String> hosts = new ArrayList<>(List.of("foo.com", "bar.com", "baz.com"));
         reg.update(foo, hosts);
-        assertEquals(3, reg.getHostsForKey(foo).size());
+        assertEquals(3, reg.getHosts(foo).size());
         hosts.remove(2);
-        assertEquals(3, reg.getHostsForKey(foo).size());
+        assertEquals(3, reg.getHosts(foo).size());
     }
 
     @Test
@@ -90,8 +90,8 @@ public class HostRegistryTest {
     }
 
     private void assertGetKey(HostRegistry reg, String host, ApplicationId expectedKey) {
-        assertNotNull(reg.getKeyForHost(host));
-        assertEquals(expectedKey, reg.getKeyForHost(host));
+        assertNotNull(reg.getApplicationId(host));
+        assertEquals(expectedKey, reg.getApplicationId(host));
     }
 
 }
diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java
index 3a9c2b1f1e7..fb31cbcdd84 100644
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java
@@ -5,9 +5,7 @@ import com.yahoo.config.model.api.ApplicationInfo;
 import com.yahoo.config.model.api.HostInfo;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
-import java.util.logging.Level;
 import com.yahoo.vespa.service.monitor.DuperModelListener;
-
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -15,6 +13,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -23,7 +22,7 @@ import java.util.logging.Logger;
  * @author hakonhall
  */
 public class DuperModel {
-    private static Logger logger = Logger.getLogger(DuperModel.class.getName());
+    private static final Logger logger = Logger.getLogger(DuperModel.class.getName());
 
     private final Map<ApplicationId, ApplicationInfo> applicationsById = new HashMap<>();
     private final Map<HostName, ApplicationId> idsByHostname = new HashMap<>();