Skip to content

Skip suspended hosts when fetching deployment metrics #36045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bjormel wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Skip suspended hosts when fetching deployment metrics #36045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions config-provisioning/src/main/java/com/yahoo/config/provision/NodeSuspensionProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.config.provision;

import java.util.Set;

/**
* Provides the set of hostnames that are currently suspended for a given application.
* Implemented in hosted Vespa using the node repository orchestrator.
*
* @author bjormel
*/
public interface NodeSuspensionProvider {

NodeSuspensionProvider EMPTY = __ -> Set.of();

/** Returns the hostnames of all nodes that are currently suspended for the given application. Never null; returns an empty set if none are suspended. */
Set<String> suspendedHosts(ApplicationId applicationId);

}
32 changes: 26 additions & 6 deletions configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.yahoo.collections.Pair;
import com.yahoo.component.Version;
import com.yahoo.component.annotation.Inject;
import com.yahoo.component.provider.ComponentRegistry;
import com.yahoo.config.FileReference;
import com.yahoo.config.application.api.ApplicationFile;
import com.yahoo.config.application.api.ApplicationMetaData;
Expand All @@ -31,6 +32,7 @@
import com.yahoo.config.provision.RegionName;
import com.yahoo.config.provision.SystemName;
import com.yahoo.config.provision.TenantName;
import com.yahoo.config.provision.NodeSuspensionProvider;
import com.yahoo.config.provision.Zone;
import com.yahoo.config.provision.exception.ActivationConflictException;
import com.yahoo.container.jdisc.HttpResponse;
Expand Down Expand Up @@ -77,6 +79,7 @@
import com.yahoo.vespa.config.server.http.v2.PrepareResult;
import com.yahoo.vespa.config.server.http.v2.response.DeploymentMetricsResponse;
import com.yahoo.vespa.config.server.http.v2.response.SearchNodeMetricsResponse;
import com.yahoo.vespa.config.server.metrics.ClusterDeploymentMetricsRetriever;
import com.yahoo.vespa.config.server.metrics.DeploymentMetricsRetriever;
import com.yahoo.vespa.config.server.metrics.SearchNodeMetricsRetriever;
import com.yahoo.vespa.config.server.provision.HostProvisionerProvider;
Expand Down Expand Up @@ -165,6 +168,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
private final ClusterReindexingStatusClient clusterReindexingStatusClient;
private final ActiveTokenFingerprints activeTokenFingerprints;
private final FlagSource flagSource;
private final DeploymentMetricsRetriever deploymentMetricsRetriever;

@Inject
public ApplicationRepository(TenantRepository tenantRepository,
Expand All @@ -177,11 +181,12 @@ public ApplicationRepository(TenantRepository tenantRepository,
TesterClient testerClient,
HealthCheckerProvider healthCheckers,
Metric metric,
FlagSource flagSource) {
FlagSource flagSource,
ComponentRegistry<NodeSuspensionProvider> nodeSuspensionProviders) {
this(tenantRepository,
hostProvisionerProvider.getHostProvisioner(),
infraDeployerProvider.getInfraDeployer(),
configConvergenceChecker,
configConvergenceChecker,
configStateChecker,
httpProxy,
EndpointsChecker.of(healthCheckers.getHealthChecker()),
Expand All @@ -193,7 +198,20 @@ public ApplicationRepository(TenantRepository tenantRepository,
new SecretStoreValidator(),
new DefaultClusterReindexingStatusClient(),
new ActiveTokenFingerprintsClient(),
flagSource);
flagSource,
new DeploymentMetricsRetriever(new ClusterDeploymentMetricsRetriever(),
nodeSuspensionProvider(nodeSuspensionProviders)));
}

private static NodeSuspensionProvider nodeSuspensionProvider(ComponentRegistry<NodeSuspensionProvider> registry) {
var providers = registry.allComponents();
if (providers.size() > 1) {
String providerDescriptions = providers.stream()
.map(provider -> provider.getClass().getName())
.collect(Collectors.joining(", "));
throw new IllegalArgumentException("Multiple NodeSuspensionProvider components registered: " + providerDescriptions);
}
return providers.isEmpty() ? NodeSuspensionProvider.EMPTY : providers.get(0);
}

private ApplicationRepository(TenantRepository tenantRepository,
Expand All @@ -211,7 +229,8 @@ private ApplicationRepository(TenantRepository tenantRepository,
SecretStoreValidator secretStoreValidator,
ClusterReindexingStatusClient clusterReindexingStatusClient,
ActiveTokenFingerprints activeTokenFingerprints,
FlagSource flagSource) {
FlagSource flagSource,
DeploymentMetricsRetriever deploymentMetricsRetriever) {
this.tenantRepository = Objects.requireNonNull(tenantRepository);
this.hostProvisioner = Objects.requireNonNull(hostProvisioner);
this.infraDeployer = Objects.requireNonNull(infraDeployer);
Expand All @@ -228,6 +247,7 @@ private ApplicationRepository(TenantRepository tenantRepository,
this.clusterReindexingStatusClient = Objects.requireNonNull(clusterReindexingStatusClient);
this.activeTokenFingerprints = Objects.requireNonNull(activeTokenFingerprints);
this.flagSource = flagSource;
this.deploymentMetricsRetriever = Objects.requireNonNull(deploymentMetricsRetriever);
}

// Should be used by tests only (first constructor in this class makes sure we use injectable components where possible)
Expand Down Expand Up @@ -327,7 +347,8 @@ public ApplicationRepository build() {
secretStoreValidator,
ClusterReindexingStatusClient.DUMMY_INSTANCE,
__ -> activeTokens,
flagSource);
flagSource,
new DeploymentMetricsRetriever());
}

}
Expand Down Expand Up @@ -983,7 +1004,6 @@ public SearchNodeMetricsResponse getSearchNodeMetrics(ApplicationId applicationI

public DeploymentMetricsResponse getDeploymentMetrics(ApplicationId applicationId) {
Application application = getApplication(applicationId);
DeploymentMetricsRetriever deploymentMetricsRetriever = new DeploymentMetricsRetriever();
return deploymentMetricsRetriever.getMetrics(application);
}

Expand Down
18 changes: 14 additions & 4 deletions ...erver/src/main/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetriever.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@

import com.yahoo.config.model.api.HostInfo;
import com.yahoo.config.model.api.ServiceInfo;
import com.yahoo.config.provision.NodeSuspensionProvider;
import com.yahoo.vespa.config.server.application.Application;
import com.yahoo.vespa.config.server.http.v2.response.DeploymentMetricsResponse;
import java.net.URI;
import java.util.Collection;
import java.util.Set;
import java.util.function.Predicate;

/**
Expand All @@ -18,28 +20,36 @@
public class DeploymentMetricsRetriever {

private final ClusterDeploymentMetricsRetriever metricsRetriever;
private final NodeSuspensionProvider nodeSuspensionProvider;

public DeploymentMetricsRetriever() {
this(new ClusterDeploymentMetricsRetriever());
this(new ClusterDeploymentMetricsRetriever(), NodeSuspensionProvider.EMPTY);
}

public DeploymentMetricsRetriever(ClusterDeploymentMetricsRetriever metricsRetriever) {
this(metricsRetriever, NodeSuspensionProvider.EMPTY);
}

public DeploymentMetricsRetriever(ClusterDeploymentMetricsRetriever metricsRetriever,
NodeSuspensionProvider nodeSuspensionProvider) {
this.metricsRetriever = metricsRetriever;
this.nodeSuspensionProvider = nodeSuspensionProvider;
}

public DeploymentMetricsResponse getMetrics(Application application) {
var hosts = getHostsOfApplication(application);
var suspendedHostnames = nodeSuspensionProvider.suspendedHosts(application.getId());
var hosts = getHostsOfApplication(application, suspendedHostnames);
var clusterMetrics = metricsRetriever.requestMetricsGroupedByCluster(hosts);
return new DeploymentMetricsResponse(application.getId(), clusterMetrics);
}

private static Collection<URI> getHostsOfApplication(Application application) {
private static Collection<URI> getHostsOfApplication(Application application, Set<String> suspendedHostnames) {
return application.getModel().getHosts().stream()
.filter(host -> host.getServices().stream().noneMatch(isLogserver()))
.filter(host -> !suspendedHostnames.contains(host.getHostname()))
.map(HostInfo::getHostname)
.map(DeploymentMetricsRetriever::createMetricsProxyURI)
.toList();

}

private static Predicate<ServiceInfo> isLogserver() {
Expand Down
36 changes: 36 additions & 0 deletions ...r/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.yahoo.config.model.api.ServiceInfo;
import com.yahoo.config.provision.AllocatedHosts;
import com.yahoo.config.provision.ApplicationId;
import com.yahoo.config.provision.NodeSuspensionProvider;
import com.yahoo.vespa.config.ConfigKey;
import com.yahoo.vespa.config.buildergen.ConfigDefinition;
import com.yahoo.vespa.config.server.application.Application;
Expand All @@ -22,6 +23,7 @@
import java.util.Set;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

/**
* @author olaa
Expand All @@ -41,6 +43,40 @@ public void getMetrics() {
assertEquals(2, mockMetricsRetriever.hosts.size()); // Verify that logserver was ignored
}

@Test
public void suspendedHostsAreFiltered() {
MockModel mockModel = new MockModel(mockHosts());
MockDeploymentMetricsRetriever mockMetricsRetriever = new MockDeploymentMetricsRetriever();
ApplicationId applicationId = ApplicationId.fromSerializedForm("tenant:app:instance");
Application application = new Application(mockModel, null, 0,
null, null, applicationId);

NodeSuspensionProvider suspensionProvider = id -> Set.of("host1");
DeploymentMetricsRetriever retriever = new DeploymentMetricsRetriever(mockMetricsRetriever, suspensionProvider);
retriever.getMetrics(application);

assertEquals(1, mockMetricsRetriever.hosts.size()); // logserver (host3) and suspended host1 are ignored
assertTrue(mockMetricsRetriever.hosts.stream().anyMatch(uri -> uri.getHost().equals("host2")));
assertTrue(mockMetricsRetriever.hosts.stream().noneMatch(uri -> uri.getHost().equals("host1")));
}

@Test
public void noSuspendedHostsWhenProviderReturnsEmptySet() {
MockModel mockModel = new MockModel(mockHosts());
MockDeploymentMetricsRetriever mockMetricsRetriever = new MockDeploymentMetricsRetriever();
ApplicationId applicationId = ApplicationId.fromSerializedForm("tenant:app:instance");
Application application = new Application(mockModel, null, 0,
null, null, applicationId);

NodeSuspensionProvider suspensionProvider = id -> Set.of();
DeploymentMetricsRetriever retriever = new DeploymentMetricsRetriever(mockMetricsRetriever, suspensionProvider);
retriever.getMetrics(application);
// With an empty suspension set, behavior should be the same as without a provider: only logserver is ignored
assertEquals(2, mockMetricsRetriever.hosts.size());
assertTrue(mockMetricsRetriever.hosts.stream().anyMatch(uri -> uri.getHost().equals("host1")));
assertTrue(mockMetricsRetriever.hosts.stream().anyMatch(uri -> uri.getHost().equals("host2")));
}

private Collection<HostInfo> mockHosts() {

HostInfo hostInfo1 = new HostInfo("host1",
Expand Down