diff options
author | Harald Musum <musum@verizonmedia.com> | 2023-02-24 10:39:56 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-02-24 10:39:56 +0100 |
commit | 45bf69ac96e892a8fd392b3a7a16aab0cf6d59e5 (patch) | |
tree | 33ad9386d95115bb9fa3b38c32d5b050644738cf | |
parent | 36c0d79e750e1c7b32dcbc6e294a462a8d691bae (diff) |
Revert "Cleanup HostRegistry, no functional changes"
6 files changed, 62 insertions, 49 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 19f41b514ec..5831cb3e75f 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.removeHosts(applicationId); + hostRegistry.removeHostsForKey(applicationId); configActivationListenersOnRemove(applicationId); tenantMetricUpdater.setApplications(applicationMapper.numApplications()); metrics.removeMetricUpdater(Metrics.createDimensions(applicationId)); @@ -277,16 +277,17 @@ public class TenantApplications implements RequestHandler, HostValidator { } private void configActivationListenersOnRemove(ApplicationId applicationId) { - configActivationListener.hostsUpdated(applicationId, hostRegistry.getHosts(applicationId)); + configActivationListener.hostsUpdated(applicationId, hostRegistry.getHostsForKey(applicationId)); configActivationListener.applicationRemoved(applicationId); } private void setActiveApp(ApplicationSet applicationSet) { - ApplicationId applicationId = applicationSet.getId(); - hostRegistry.update(applicationId, applicationSet.getAllHosts()); + ApplicationId id = applicationSet.getId(); + Collection<String> hostsForApp = applicationSet.getAllHosts(); + hostRegistry.update(id, hostsForApp); applicationSet.updateHostMetrics(); tenantMetricUpdater.setApplications(applicationMapper.numApplications()); - applicationMapper.register(applicationId, applicationSet); + applicationMapper.register(id, applicationSet); } @Override @@ -376,7 +377,7 @@ public class TenantApplications implements RequestHandler, HostValidator { @Override public ApplicationId resolveApplicationId(String hostName) { - return hostRegistry.getApplicationId(hostName); + return hostRegistry.getKeyForHost(hostName); } @Override @@ -402,7 +403,7 @@ public class TenantApplications implements RequestHandler, HostValidator { } public ApplicationId getApplicationIdForHostName(String hostname) { - return hostRegistry.getApplicationId(hostname); + return hostRegistry.getKeyForHost(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 1a7408d6251..b89f3bba835 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,26 +9,35 @@ 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 hostname and ApplicationId + * 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. * * @author Ulf Lilleengen */ public class HostRegistry implements HostValidator { - private final Map<String, ApplicationId> host2ApplicationId = new ConcurrentHashMap<>(); + private static final Logger log = Logger.getLogger(HostRegistry.class.getName()); - public ApplicationId getApplicationId(String hostName) { - return host2ApplicationId.get(hostName); + private final Map<String, ApplicationId> host2KeyMap = new ConcurrentHashMap<>(); + + public ApplicationId getKeyForHost(String hostName) { + return host2KeyMap.get(hostName); } - public synchronized void update(ApplicationId applicationId, Collection<String> newHosts) { - verifyHosts(applicationId, newHosts); - Collection<String> removedHosts = findRemovedHosts(newHosts, getHosts(applicationId)); + 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); removeHosts(removedHosts); - addHosts(applicationId, newHosts); + addHosts(key, newHosts); } @Override @@ -36,47 +45,49 @@ 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 '" + host2ApplicationId.get(host) + "'"); + "', but the host is already taken by '" + host2KeyMap.get(host) + "'"); } } } - public synchronized void removeHosts(ApplicationId applicationId) { - host2ApplicationId.entrySet().removeIf(entry -> entry.getValue().equals(applicationId)); + public synchronized void removeHostsForKey(ApplicationId key) { + host2KeyMap.entrySet().removeIf(entry -> entry.getValue().equals(key)); } - public synchronized void removeHosts(TenantName tenantName) { - host2ApplicationId.entrySet().removeIf(entry -> entry.getValue().tenant().equals(tenantName)); + public synchronized void removeHostsForKey(TenantName key) { + host2KeyMap.entrySet().removeIf(entry -> entry.getValue().tenant().equals(key)); } public synchronized Collection<String> getAllHosts() { - return Collections.unmodifiableCollection(new ArrayList<>(host2ApplicationId.keySet())); + return Collections.unmodifiableCollection(new ArrayList<>(host2KeyMap.keySet())); } - public synchronized Collection<String> getHosts(ApplicationId applicationId) { - return host2ApplicationId.entrySet().stream() - .filter(entry -> entry.getValue().equals(applicationId)) - .map(Map.Entry::getKey) - .collect(Collectors.toSet()); + public synchronized Collection<String> getHostsForKey(ApplicationId key) { + return host2KeyMap.entrySet().stream() + .filter(entry -> entry.getValue().equals(key)) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); } - private boolean hostAlreadyTaken(String host, ApplicationId applicationId) { - return host2ApplicationId.containsKey(host) && !applicationId.equals(host2ApplicationId.get(host)); + private boolean hostAlreadyTaken(String host, ApplicationId key) { + return host2KeyMap.containsKey(host) && !key.equals(host2KeyMap.get(host)); } - private static Collection<String> findRemovedHosts(Collection<String> newHosts, Collection<String> previousHosts) { + private static Collection<String> getRemovedHosts(Collection<String> newHosts, Collection<String> previousHosts) { return Collections2.filter(previousHosts, host -> !newHosts.contains(host)); } - private void removeHosts(Collection<String> hosts) { - for (String host : hosts) { - host2ApplicationId.remove(host); + private void removeHosts(Collection<String> removedHosts) { + for (String host : removedHosts) { + log.log(Level.FINE, () -> "Removing " + host); + host2KeyMap.remove(host); } } - private void addHosts(ApplicationId key, Collection<String> hosts) { - for (String host : hosts) { - host2ApplicationId.put(host, key); + private void addHosts(ApplicationId key, Collection<String> newHosts) { + for (String host : newHosts) { + log.log(Level.FINE, () -> "Adding " + host); + host2KeyMap.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 aac34238b90..be4738258d8 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.getApplicationId(hostname); + ApplicationId applicationId = hostRegistry.getKeyForHost(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.removeHosts(tenant); + hostRegistry.removeHostsForKey(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 21f7354401f..536a446df2f 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.getApplicationId(hostname); + ApplicationId applicationId = hostRegistry.getKeyForHost(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 646017a498e..df00d28134f 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.getApplicationId("foo.com")); + assertNull(reg.getKeyForHost("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.getApplicationId("foo.com")); + assertNull(reg.getKeyForHost("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.removeHosts(foo); + reg.removeHostsForKey(foo); assertTrue(reg.getAllHosts().isEmpty()); - assertNull(reg.getApplicationId("foo.com")); - assertNull(reg.getApplicationId("bar.com")); + assertNull(reg.getKeyForHost("foo.com")); + assertNull(reg.getKeyForHost("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.getHosts(foo).size()); + assertEquals(3, reg.getHostsForKey(foo).size()); hosts.remove(2); - assertEquals(3, reg.getHosts(foo).size()); + assertEquals(3, reg.getHostsForKey(foo).size()); } @Test @@ -90,8 +90,8 @@ public class HostRegistryTest { } private void assertGetKey(HostRegistry reg, String host, ApplicationId expectedKey) { - assertNotNull(reg.getApplicationId(host)); - assertEquals(expectedKey, reg.getApplicationId(host)); + assertNotNull(reg.getKeyForHost(host)); + assertEquals(expectedKey, reg.getKeyForHost(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 fb31cbcdd84..3a9c2b1f1e7 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,7 +5,9 @@ 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; @@ -13,7 +15,6 @@ 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; /** @@ -22,7 +23,7 @@ import java.util.logging.Logger; * @author hakonhall */ public class DuperModel { - private static final Logger logger = Logger.getLogger(DuperModel.class.getName()); + private static Logger logger = Logger.getLogger(DuperModel.class.getName()); private final Map<ApplicationId, ApplicationInfo> applicationsById = new HashMap<>(); private final Map<HostName, ApplicationId> idsByHostname = new HashMap<>(); |