summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorValerij Fredriksen <freva@users.noreply.github.com>2023-02-24 14:51:02 +0100
committerGitHub <noreply@github.com>2023-02-24 14:51:02 +0100
commitc0db5d5bafc2ff638f6175bbcbb19b42d18e14c3 (patch)
tree33ad9386d95115bb9fa3b38c32d5b050644738cf
parent36c0d79e750e1c7b32dcbc6e294a462a8d691bae (diff)
parent45bf69ac96e892a8fd392b3a7a16aab0cf6d59e5 (diff)
Merge pull request #26172 from vespa-engine/revert-26170-hmusum/configserver-cleanup-1b
Revert "Cleanup HostRegistry, no functional changes"
-rw-r--r--configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java15
-rw-r--r--configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java67
-rw-r--r--configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java4
-rw-r--r--configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java2
-rw-r--r--configserver/src/test/java/com/yahoo/vespa/config/server/host/HostRegistryTest.java18
-rw-r--r--service-monitor/src/main/java/com/yahoo/vespa/service/duper/DuperModel.java5
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<>();