From 39b0457169616f1c25dbd85f58951cc7b77dd5c4 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Thu, 12 Mar 2020 10:54:33 +0100 Subject: Reduce application reads --- .../yahoo/vespa/hosted/controller/Application.java | 2 +- .../hosted/controller/ApplicationController.java | 8 ++-- .../vespa/hosted/controller/RoutingController.java | 48 +++++++++++----------- .../restapi/application/ApplicationApiHandler.java | 5 +-- 4 files changed, 32 insertions(+), 31 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java index 5f1543ca143..0955a5388ce 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java @@ -30,7 +30,7 @@ import java.util.function.Function; import java.util.stream.Collectors; /** - * An application. Belongs to a {@link Tenant}, and may have many {@link Instance}s. + * An application. Belongs to a {@link Tenant}, and may have multiple {@link Instance}s. * * This is immutable. * diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 49e8ff22bfc..a382d357e49 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -320,7 +320,7 @@ public class ApplicationController { endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(instance, zone); - endpoints = controller.routing().registerEndpointsInDns(applicationPackage.deploymentSpec(), instance, zone); + endpoints = controller.routing().registerEndpointsInDns(application.get(), job.application().instance(), zone); } // Release application lock while doing the deployment, which is a lengthy task. // Carry out deployment without holding the application lock. @@ -393,7 +393,7 @@ public class ApplicationController { endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(application.get().require(instance), zone); - endpoints = controller.routing().registerEndpointsInDns(applicationPackage.deploymentSpec(), application.get().require(instanceId.instance()), zone); + endpoints = controller.routing().registerEndpointsInDns(application.get(), instance, zone); } // Release application lock while doing the deployment, which is a lengthy task. // Carry out deployment without holding the application lock. @@ -565,7 +565,7 @@ public class ApplicationController { throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments: " + deployments); for (Instance instance : application.get().instances().values()) { - controller.routing().removeEndpointsInDns(instance); + controller.routing().removeEndpointsInDns(application.get(), instance.name()); application = application.without(instance.name()); } @@ -600,7 +600,7 @@ public class ApplicationController { && application.get().deploymentSpec().instanceNames().contains(instanceId.instance())) throw new IllegalArgumentException("Can not delete '" + instanceId + "', which is specified in 'deployment.xml'; remove it there first"); - controller.routing().removeEndpointsInDns(application.get().require(instanceId.instance())); + controller.routing().removeEndpointsInDns(application.get(), instanceId.instance()); curator.writeApplication(application.without(instanceId.instance()).get()); controller.jobController().collectGarbage(); log.info("Deleted " + instanceId); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 24f5f0a8659..6634b7ef81e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.controller; import com.google.common.base.Suppliers; import com.yahoo.component.Version; import com.yahoo.config.application.api.DeploymentInstanceSpec; -import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; @@ -49,8 +48,8 @@ import java.util.TreeMap; import java.util.stream.Collectors; /** - * The routing controller encapsulates state and methods for inspecting and manipulating DNS-level routing of traffic - * in a system. + * The routing controller encapsulates state and methods for inspecting and manipulating deployment endpoints in a + * hosted Vespa system. * * The one stop shop for all your routing needs! * @@ -110,22 +109,23 @@ public class RoutingController { /** Returns global-scoped endpoints for given instance */ public EndpointList endpointsOf(ApplicationId instance) { - return endpointsOf(controller.applications().requireInstance(instance)); + return endpointsOf(controller.applications().requireApplication(TenantAndApplicationId.from(instance)), + instance.instance()); } /** Returns global-scoped endpoints for given instance */ // TODO(mpolden): Add a endpointsOf(Instance, DeploymentId) variant of this that only returns global endpoint of // which deployment is a member - public EndpointList endpointsOf(Instance instance) { + public EndpointList endpointsOf(Application application, InstanceName instanceName) { var endpoints = new LinkedHashSet(); // Add global endpoints provided by rotations - var application = Suppliers.memoize(() -> controller.applications().requireApplication(TenantAndApplicationId.from(instance.id()))); + var instance = application.require(instanceName); for (var rotation : instance.rotations()) { var deployments = rotation.regions().stream() .map(region -> new DeploymentId(instance.id(), ZoneId.from(Environment.prod, region))) .collect(Collectors.toList()); EndpointList.global(RoutingId.of(instance.id(), rotation.endpointId()), - controller.system(), routingMethodsAvailableTo(deployments, application.get())) + controller.system(), routingMethodsOfAll(deployments, application)) .requiresRotation() .forEach(endpoints::add); } @@ -140,7 +140,7 @@ public class RoutingController { } } deploymentsByRoutingId.forEach((routingId, deployments) -> { - EndpointList.global(routingId, controller.system(), routingMethodsAvailableTo(deployments, application.get())) + EndpointList.global(routingId, controller.system(), routingMethodsOfAll(deployments, application)) .not().requiresRotation() .forEach(endpoints::add); }); @@ -201,14 +201,16 @@ public class RoutingController { * * @return the registered endpoints */ - public Set registerEndpointsInDns(DeploymentSpec deploymentSpec, Instance instance, ZoneId zone) { + public Set registerEndpointsInDns(Application application, InstanceName instanceName, ZoneId zone) { + var instance = application.require(instanceName); var containerEndpoints = new HashSet(); - boolean registerLegacyNames = deploymentSpec.instance(instance.name()) - .flatMap(DeploymentInstanceSpec::globalServiceId) - .isPresent(); + boolean registerLegacyNames = application.deploymentSpec().instance(instanceName) + .flatMap(DeploymentInstanceSpec::globalServiceId) + .isPresent(); for (var assignedRotation : instance.rotations()) { var names = new ArrayList(); - var endpoints = endpointsOf(instance).named(assignedRotation.endpointId()).requiresRotation(); + var endpoints = endpointsOf(application, instanceName).named(assignedRotation.endpointId()) + .requiresRotation(); // Skip rotations which do not apply to this zone. Legacy names always point to all zones if (!registerLegacyNames && !assignedRotation.regions().contains(zone.region())) { @@ -239,16 +241,16 @@ public class RoutingController { } /** Remove endpoints in DNS for all rotations assigned to given instance */ - public void removeEndpointsInDns(Instance instance) { - endpointsOf(instance).requiresRotation() - .forEach(endpoint -> controller.nameServiceForwarder() - .removeRecords(Record.Type.CNAME, - RecordName.from(endpoint.dnsName()), - Priority.normal)); + public void removeEndpointsInDns(Application application, InstanceName instanceName) { + endpointsOf(application, instanceName).requiresRotation() + .forEach(endpoint -> controller.nameServiceForwarder() + .removeRecords(Record.Type.CNAME, + RecordName.from(endpoint.dnsName()), + Priority.normal)); } - /** Returns the routing methods that are available across given deployments */ - private List routingMethodsAvailableTo(List deployments, Application application) { + /** Returns the routing methods that are available across all given deployments */ + private List routingMethodsOfAll(List deployments, Application application) { var deploymentsByMethod = new HashMap>(); for (var deployment : deployments) { for (var method : controller.zoneRegistry().routingMethods(deployment.zoneId())) { @@ -257,8 +259,8 @@ public class RoutingController { } } var routingMethods = new ArrayList(); - deploymentsByMethod.forEach((method, deploymentsOfMethod) -> { - if (deploymentsOfMethod.containsAll(deployments)) { + deploymentsByMethod.forEach((method, supportedDeployments) -> { + if (supportedDeployments.containsAll(deployments)) { if (method.isDirect() && !canRouteDirectlyTo(deployments, application)) return; routingMethods.add(method); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 69c19caf086..05a4c34746c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -1066,9 +1066,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { response.setString("instance", deploymentId.applicationId().instance().value()); // pointless response.setString("environment", deploymentId.zoneId().environment().value()); response.setString("region", deploymentId.zoneId().region().value()); - var application = controller.applications().requireApplication(TenantAndApplicationId.from(deploymentId.applicationId())); - var instance = application.instances().get(deploymentId.applicationId().instance()); // Add zone endpoints var endpointArray = response.setArray("endpoints"); @@ -1081,7 +1079,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { } // Add global endpoints if (deploymentId.zoneId().environment().isProduction()) { // Global endpoints can only point to production deployments - for (var endpoint : controller.routing().endpointsOf(instance).not().legacy()) { + for (var endpoint : controller.routing().endpointsOf(application, deploymentId.applicationId().instance()).not().legacy()) { // TODO(mpolden): Pass cluster name. Cluster that a global endpoint points to is not available at this level. toSlime(endpoint, "", endpointArray.addObject()); } @@ -1102,6 +1100,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler { application.projectId().ifPresent(i -> response.setString("screwdriverId", String.valueOf(i))); sourceRevisionToSlime(deployment.applicationVersion().source(), response); + var instance = application.instances().get(deploymentId.applicationId().instance()); if (instance != null) { if (!instance.rotations().isEmpty() && deployment.zone().environment() == Environment.prod) toSlime(instance.rotations(), instance.rotationStatus(), deployment, response); -- cgit v1.2.3