From 5179fc9ac037d3bcc3cdc1a044caef8d5ee5a84a Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 22 Feb 2023 09:08:12 +0100 Subject: Cleanup HostRegistry, no functional changes --- .../server/application/TenantApplications.java | 15 +++-- .../vespa/config/server/host/HostRegistry.java | 67 +++++++++------------- .../yahoo/vespa/config/server/rpc/RpcServer.java | 4 +- .../rpc/security/MultiTenantRpcAuthorizer.java | 2 +- .../vespa/config/server/host/HostRegistryTest.java | 18 +++--- .../com/yahoo/vespa/service/duper/DuperModel.java | 5 +- 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 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 host2ApplicationId = new ConcurrentHashMap<>(); - private final Map 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 newHosts) { - verifyHosts(key, newHosts); - Collection currentHosts = getHostsForKey(key); - log.log(Level.FINE, () -> "Setting hosts for key '" + key + "', " + - "newHosts: " + newHosts + ", " + - "currentHosts: " + currentHosts); - Collection removedHosts = getRemovedHosts(newHosts, currentHosts); + public synchronized void update(ApplicationId applicationId, Collection newHosts) { + verifyHosts(applicationId, newHosts); + Collection 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 getAllHosts() { - return Collections.unmodifiableCollection(new ArrayList<>(host2KeyMap.keySet())); + return Collections.unmodifiableCollection(new ArrayList<>(host2ApplicationId.keySet())); } - public synchronized Collection getHostsForKey(ApplicationId key) { - return host2KeyMap.entrySet().stream() - .filter(entry -> entry.getValue().equals(key)) - .map(Map.Entry::getKey) - .collect(Collectors.toSet()); + public synchronized Collection 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 getRemovedHosts(Collection newHosts, Collection previousHosts) { + private static Collection findRemovedHosts(Collection newHosts, Collection previousHosts) { return Collections2.filter(previousHosts, host -> !newHosts.contains(host)); } - private void removeHosts(Collection removedHosts) { - for (String host : removedHosts) { - log.log(Level.FINE, () -> "Removing " + host); - host2KeyMap.remove(host); + private void removeHosts(Collection hosts) { + for (String host : hosts) { + host2ApplicationId.remove(host); } } - private void addHosts(ApplicationId key, Collection newHosts) { - for (String host : newHosts) { - log.log(Level.FINE, () -> "Adding " + host); - host2KeyMap.put(host, key); + private void addHosts(ApplicationId key, Collection 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 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 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 applicationsById = new HashMap<>(); private final Map idsByHostname = new HashMap<>(); -- cgit v1.2.3