diff options
author | Martin Polden <mpolden@mpolden.no> | 2020-07-09 11:07:32 +0200 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2020-07-09 14:51:26 +0200 |
commit | ae0051eb76a2600bdcce5a119de06f367be7d164 (patch) | |
tree | cdd7137df82d32f3a72fb268a36b059c9a82a7d0 | |
parent | fa1346f02f2588de9e04fee801a75c63b0cc05db (diff) |
Control maintenance jobs with feature flag
38 files changed, 200 insertions, 286 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java index b0ca6ba67f5..7c983ab48a0 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java @@ -42,7 +42,7 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { Duration interval, ConfigserverConfig configserverConfig, FlagSource flagSource) { - super(applicationRepository, curator, interval, interval); + super(applicationRepository, curator, flagSource, interval, interval); this.applicationRepository = applicationRepository; this.configserverConfig = configserverConfig; diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java index 7b4664e040c..5369bbef366 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintainer.java @@ -2,18 +2,18 @@ package com.yahoo.vespa.config.server.maintenance; import com.yahoo.concurrent.maintenance.JobControl; +import com.yahoo.concurrent.maintenance.JobControlState; import com.yahoo.concurrent.maintenance.Maintainer; -import com.yahoo.concurrent.maintenance.StringSetSerializer; import com.yahoo.path.Path; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.ListFlag; import java.time.Duration; -import java.util.HashSet; import java.util.Set; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A maintainer is some job which runs at a fixed interval to perform some maintenance task in the config server. @@ -24,47 +24,28 @@ public abstract class ConfigServerMaintainer extends Maintainer { protected final ApplicationRepository applicationRepository; - ConfigServerMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration initialDelay, Duration interval) { - super(null, interval, initialDelay, new JobControl(new JobControlDb(curator))); + ConfigServerMaintainer(ApplicationRepository applicationRepository, Curator curator, FlagSource flagSource, + Duration initialDelay, Duration interval) { + super(null, interval, initialDelay, new JobControl(new JobControlFlags(curator, flagSource))); this.applicationRepository = applicationRepository; } - private static class JobControlDb implements JobControl.Db { - - private static final Logger log = Logger.getLogger(JobControlDb.class.getName()); + private static class JobControlFlags implements JobControlState { private static final Path root = Path.fromString("/configserver/v1/"); private static final Path lockRoot = root.append("locks"); - private static final Path inactiveJobsPath = root.append("inactiveJobs"); private final Curator curator; - private final StringSetSerializer serializer = new StringSetSerializer(); + private final ListFlag<String> inactiveJobsFlag; - public JobControlDb(Curator curator) { + public JobControlFlags(Curator curator, FlagSource flagSource) { this.curator = curator; + this.inactiveJobsFlag = Flags.INACTIVE_MAINTENANCE_JOBS.bindTo(flagSource); } @Override public Set<String> readInactiveJobs() { - try { - return curator.getData(inactiveJobsPath) - .filter(data -> data.length > 0) - .map(serializer::fromJson).orElseGet(HashSet::new); - } catch (RuntimeException e) { - log.log(Level.WARNING, "Error reading inactive jobs, deleting inactive state"); - writeInactiveJobs(Set.of()); - return new HashSet<>(); - } - } - - @Override - public void writeInactiveJobs(Set<String> inactiveJobs) { - curator.set(inactiveJobsPath, serializer.toJson(inactiveJobs)); - } - - @Override - public Mutex lockInactiveJobs() { - return curator.lock(lockRoot.append("inactiveJobsLock"), Duration.ofSeconds(1)); + return Set.copyOf(inactiveJobsFlag.value()); } @Override diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java index b948da05556..a6585be391c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java @@ -35,8 +35,8 @@ public class ConfigServerMaintenance extends AbstractComponent { DefaultTimes defaults = new DefaultTimes(configserverConfig); // TODO: Disabled until we have application metadata //tenantsMaintainer = new TenantsMaintainer(applicationRepository, curator, defaults.tenantsMaintainerInterval); - fileDistributionMaintainer = new FileDistributionMaintainer(applicationRepository, curator, defaults.defaultInterval, configserverConfig); - sessionsMaintainer = new SessionsMaintainer(applicationRepository, curator, Duration.ofMinutes(1)); + fileDistributionMaintainer = new FileDistributionMaintainer(applicationRepository, curator, defaults.defaultInterval, configserverConfig, flagSource); + sessionsMaintainer = new SessionsMaintainer(applicationRepository, curator, Duration.ofMinutes(1), flagSource); applicationPackageMaintainer = new ApplicationPackageMaintainer(applicationRepository, curator, Duration.ofMinutes(1), configserverConfig, flagSource); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java index 604212b0ae8..ed323438e3f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java @@ -5,6 +5,7 @@ import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.flags.FlagSource; import java.io.File; import java.time.Duration; @@ -25,8 +26,9 @@ public class FileDistributionMaintainer extends ConfigServerMaintainer { FileDistributionMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, - ConfigserverConfig configserverConfig) { - super(applicationRepository, curator, interval, interval); + ConfigserverConfig configserverConfig, + FlagSource flagSource) { + super(applicationRepository, curator, flagSource, interval, interval); this.applicationRepository = applicationRepository; this.configserverConfig = configserverConfig; this.fileReferencesDir = new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir())); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java index ec06336c80a..a799d22e771 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.config.server.maintenance; import com.yahoo.log.LogLevel; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.flags.FlagSource; import java.time.Duration; @@ -17,10 +18,10 @@ import java.time.Duration; public class SessionsMaintainer extends ConfigServerMaintainer { private final boolean hostedVespa; - SessionsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval) { + SessionsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, FlagSource flagSource) { // Start this maintainer immediately. It frees disk space, so if disk goes full and config server // restarts this makes sure that cleanup will happen as early as possible - super(applicationRepository, curator, Duration.ZERO, interval); + super(applicationRepository, curator, flagSource, Duration.ZERO, interval); this.hostedVespa = applicationRepository.configserverConfig().hostedVespa(); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java index d3865f287cd..9a81d9f7547 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.maintenance; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.flags.FlagSource; import java.time.Clock; import java.time.Duration; @@ -19,8 +20,9 @@ public class TenantsMaintainer extends ConfigServerMaintainer { private final Duration ttlForUnusedTenant; private final Clock clock; - TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, Clock clock) { - super(applicationRepository, curator, interval, interval); + TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, FlagSource flagSource, + Duration interval, Clock clock) { + super(applicationRepository, curator, flagSource, interval, interval); this.ttlForUnusedTenant = defaultTtlForUnusedTenant; this.clock = clock; } @@ -29,4 +31,5 @@ public class TenantsMaintainer extends ConfigServerMaintainer { protected void maintain() { applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, clock.instant()); } + } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java index 53326a89293..a24367d250b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java @@ -9,6 +9,7 @@ import com.yahoo.test.ManualClock; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.config.server.tenant.TenantRepository; +import com.yahoo.vespa.flags.InMemoryFlagSource; import org.junit.Test; import java.io.File; @@ -39,7 +40,7 @@ public class TenantsMaintainerTest { assertNotNull(tenantRepository.getTenant(shouldNotBeDeleted)); clock.advance(TenantsMaintainer.defaultTtlForUnusedTenant.plus(Duration.ofDays(1))); - new TenantsMaintainer(applicationRepository, tester.curator(), Duration.ofDays(1), clock).run(); + new TenantsMaintainer(applicationRepository, tester.curator(), new InMemoryFlagSource(), Duration.ofDays(1), clock).run(); tenantRepository.updateTenants(); // One tenant should now have been deleted diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 9e4600a1bdb..9750c322242 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -23,6 +23,7 @@ import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder; import com.yahoo.vespa.hosted.controller.metric.ConfigServerMetrics; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; +import com.yahoo.vespa.hosted.controller.persistence.JobControlFlags; import com.yahoo.vespa.hosted.controller.security.AccessControl; import com.yahoo.vespa.hosted.controller.versions.ControllerVersion; import com.yahoo.vespa.hosted.controller.versions.OsVersion; @@ -112,7 +113,7 @@ public class Controller extends AbstractComponent { tenantController = new TenantController(this, curator, accessControl); routingController = new RoutingController(this, Objects.requireNonNull(rotationsConfig, "RotationsConfig cannot be null")); auditLogger = new AuditLogger(curator, clock); - jobControl = new JobControl(curator); + jobControl = new JobControl(new JobControlFlags(curator, flagSource)); this.controllerConfig = controllerConfig; // Record the version of this controller diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 3058037ccc0..0a9dd2e55a6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -5,8 +5,6 @@ import com.google.common.util.concurrent.UncheckedTimeoutException; import com.google.inject.Inject; import com.yahoo.collections.Pair; import com.yahoo.component.Version; -import com.yahoo.concurrent.maintenance.JobControl; -import com.yahoo.concurrent.maintenance.StringSetSerializer; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.TenantName; @@ -44,7 +42,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.NavigableMap; @@ -72,7 +69,7 @@ import static java.util.stream.Collectors.toUnmodifiableList; * @author mpolden * @author jonmv */ -public class CuratorDb implements JobControl.Db { +public class CuratorDb { private static final Logger log = Logger.getLogger(CuratorDb.class.getName()); private static final Duration deployLockTimeout = Duration.ofMinutes(30); @@ -89,7 +86,6 @@ public class CuratorDb implements JobControl.Db { private static final Path zoneRoutingPoliciesRoot = root.append("zoneRoutingPolicies"); private static final Path endpointCertificateRoot = root.append("applicationCertificates"); - private final StringSetSerializer stringSetSerializer = new StringSetSerializer(); private final NodeVersionSerializer nodeVersionSerializer = new NodeVersionSerializer(); private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer(nodeVersionSerializer); private final ControllerVersionSerializer controllerVersionSerializer = new ControllerVersionSerializer(); @@ -159,12 +155,6 @@ public class CuratorDb implements JobControl.Db { return curator.lock(lockRoot.append("confidenceOverrides"), defaultLockTimeout); } - @Override - public Lock lockInactiveJobs() { - return curator.lock(lockRoot.append("inactiveJobsLock"), defaultLockTimeout); - } - - @Override public Lock lockMaintenanceJob(String jobName) { try { return tryLock(lockRoot.append("maintenanceJobLocks").append(jobName)); @@ -229,25 +219,6 @@ public class CuratorDb implements JobControl.Db { } } - // -------------- Maintenance jobs ---------------------------------------- - - @Override - public Set<String> readInactiveJobs() { - try { - return read(inactiveJobsPath(), stringSetSerializer::fromJson).orElseGet(HashSet::new); - } - catch (RuntimeException e) { - log.log(Level.WARNING, "Error reading inactive jobs, deleting inactive state"); - writeInactiveJobs(Collections.emptySet()); - return new HashSet<>(); - } - } - - @Override - public void writeInactiveJobs(Set<String> inactiveJobs) { - curator.set(inactiveJobsPath(), stringSetSerializer.toJson(inactiveJobs)); - } - // -------------- Deployment orchestration -------------------------------- public double readUpgradesPerMinute() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/JobControlFlags.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/JobControlFlags.java new file mode 100644 index 00000000000..1cb23d0c515 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/JobControlFlags.java @@ -0,0 +1,37 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.concurrent.maintenance.JobControlState; +import com.yahoo.transaction.Mutex; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.ListFlag; + +import java.util.Set; + +/** + * An implementation of {@link JobControlState} that uses a feature flag to control maintenance jobs. + * + * @author mpolden + */ +public class JobControlFlags implements JobControlState { + + private final CuratorDb curator; + private final ListFlag<String> inactiveJobsFlag; + + public JobControlFlags(CuratorDb curator, FlagSource flagSource) { + this.curator = curator; + this.inactiveJobsFlag = Flags.INACTIVE_MAINTENANCE_JOBS.bindTo(flagSource); + } + + @Override + public Set<String> readInactiveJobs() { + return Set.copyOf(inactiveJobsFlag.value()); + } + + @Override + public Mutex lockMaintenanceJob(String job) { + return curator.lockMaintenanceJob(job); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java index 8b1beb91b83..10c7452a6af 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java @@ -6,16 +6,15 @@ import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.io.IOUtils; +import com.yahoo.restapi.ErrorResponse; import com.yahoo.restapi.Path; +import com.yahoo.restapi.ResourceResponse; import com.yahoo.slime.Inspector; import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.auditlog.AuditLoggingRequestHandler; import com.yahoo.vespa.hosted.controller.maintenance.ControllerMaintenance; import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; -import com.yahoo.restapi.ErrorResponse; -import com.yahoo.restapi.MessageResponse; -import com.yahoo.restapi.ResourceResponse; import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import com.yahoo.yolean.Exceptions; @@ -76,14 +75,12 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { private HttpResponse post(HttpRequest request) { Path path = new Path(request.getUri()); - if (path.matches("/controller/v1/maintenance/inactive/{jobName}")) return setActive(path.get("jobName"), false); if (path.matches("/controller/v1/jobs/upgrader/confidence/{version}")) return overrideConfidence(request, path.get("version")); return notFound(path); } private HttpResponse delete(HttpRequest request) { Path path = new Path(request.getUri()); - if (path.matches("/controller/v1/maintenance/inactive/{jobName}")) return setActive(path.get("jobName"), true); if (path.matches("/controller/v1/jobs/upgrader/confidence/{version}")) return removeConfidenceOverride(path.get("version")); return notFound(path); } @@ -100,14 +97,6 @@ public class ControllerApiHandler extends AuditLoggingRequestHandler { return new ResourceResponse(request, "auditlog", "jobs/upgrader", "maintenance"); } - private HttpResponse setActive(String jobName, boolean active) { - boolean activatingInactiveJob = active && !controller.jobControl().isActive(jobName); - if (!activatingInactiveJob && !controller.jobControl().jobs().contains(jobName)) - return ErrorResponse.notFoundError("No job named '" + jobName + "'"); - controller.jobControl().setActive(jobName, active); - return new MessageResponse((active ? "Re-activated" : "Deactivated" ) + " job '" + jobName + "'"); - } - private HttpResponse configureUpgrader(HttpRequest request) { String upgradesPerMinuteField = "upgradesPerMinute"; String targetMajorVersionField = "targetMajorVersion"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/JobsResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/JobsResponse.java index df259b2c6f3..0ac90349eaf 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/JobsResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/JobsResponse.java @@ -6,6 +6,8 @@ import com.yahoo.restapi.SlimeJsonResponse; import com.yahoo.slime.Cursor; import com.yahoo.slime.Slime; +import java.util.TreeSet; + /** * A response containing maintenance job status * @@ -26,7 +28,7 @@ public class JobsResponse extends SlimeJsonResponse { jobArray.addObject().setString("name", jobName); Cursor inactiveArray = root.setArray("inactive"); - for (String jobName : jobControl.inactiveJobs()) + for (String jobName : new TreeSet<>(jobControl.inactiveJobs())) inactiveArray.addString(jobName); return slime; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 993fb919594..41d10015411 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -19,6 +19,7 @@ import com.yahoo.vespa.hosted.controller.maintenance.JobRunnerTest; import com.yahoo.vespa.hosted.controller.maintenance.OutstandingChangeDeployer; import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger; import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; +import com.yahoo.vespa.hosted.controller.persistence.JobControlFlags; import java.time.DayOfWeek; import java.time.Duration; @@ -76,7 +77,7 @@ public class DeploymentTester { tester = controllerTester; jobs = tester.controller().jobController(); cloud = (MockTesterCloud) tester.controller().jobController().cloud(); - var jobControl = new JobControl(tester.controller().curator()); + var jobControl = new JobControl(new JobControlFlags(tester.controller().curator(), tester.controller().flagSource())); runner = new JobRunner(tester.controller(), Duration.ofDays(1), JobRunnerTest.inThreadExecutor(), new InternalStepRunner(tester.controller())); upgrader = new Upgrader(tester.controller(), maintenanceInterval, tester.curator()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java index 8029d650b75..c061bfed21a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java @@ -6,6 +6,8 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.test.ManualClock; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshot; import com.yahoo.vespa.hosted.controller.auditlog.AuditLogger; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; @@ -19,7 +21,6 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; import java.util.List; -import java.util.Set; import static org.junit.Assert.assertFalse; @@ -41,39 +42,11 @@ public class ControllerApiTest extends ControllerContainerTest { public void testControllerApi() { tester.assertResponse(authenticatedRequest("http://localhost:8080/controller/v1/", "", Request.Method.GET), new File("root.json")); - // POST deactivates a maintenance job - tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/maintenance/inactive/DeploymentExpirer", - "", Request.Method.POST), - "{\"message\":\"Deactivated job 'DeploymentExpirer'\"}", 200); + ((InMemoryFlagSource) tester.controller().flagSource()).withListFlag(Flags.INACTIVE_MAINTENANCE_JOBS.id(), List.of("DeploymentExpirer"), String.class); // GET a list of all maintenance jobs tester.assertResponse(authenticatedRequest("http://localhost:8080/controller/v1/maintenance/", "", Request.Method.GET), new File("maintenance.json")); - - // DELETE activates maintenance job - tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/maintenance/inactive/DeploymentExpirer", - "", Request.Method.DELETE), - "{\"message\":\"Re-activated job 'DeploymentExpirer'\"}", - 200); - - // DELETE fails to activate unknown maintenance job - tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/maintenance/inactive/foo", - "", Request.Method.DELETE), - "{\"error-code\":\"NOT_FOUND\",\"message\":\"No job named 'foo'\"}", - 404); - - // DELETE clears inactive flag for maintenance job that has been removed from the code base - tester.controller().curator().writeInactiveJobs(Set.of("bar")); - tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/maintenance/inactive/bar", - "", Request.Method.DELETE), - "{\"message\":\"Re-activated job 'bar'\"}", - 200); - tester.assertResponse(operatorRequest("http://localhost:8080/controller/v1/maintenance/inactive/bar", - "", Request.Method.DELETE), - "{\"error-code\":\"NOT_FOUND\",\"message\":\"No job named 'bar'\"}", - 404); - - assertFalse("Actions are logged to audit log", tester.controller().auditLogger().readLog().entries().isEmpty()); } @Test diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 6addf4b932c..038985accdb 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -123,6 +123,11 @@ public class Flags { "Otherwise it specifies the total (unallocated or not) capacity.", "Takes effect on next iteration of DynamicProvisioningMaintainer."); + public static final UnboundListFlag<String> INACTIVE_MAINTENANCE_JOBS = defineListFlag( + "inactive-maintenance-jobs", List.of(), String.class, + "The list of maintenance jobs that are inactive.", + "Takes effect immediately, but any currently running jobs will run until completion."); + public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( "default-term-wise-limit", 1.0, "Default limit for when to apply termwise query evaluation", diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index 0dd3869e234..99b6c48c90b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -17,6 +17,7 @@ import com.yahoo.config.provisioning.NodeRepositoryConfig; import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.hosted.provision.Node.State; import com.yahoo.vespa.hosted.provision.applications.Applications; import com.yahoo.vespa.hosted.provision.lb.LoadBalancer; @@ -35,6 +36,7 @@ import com.yahoo.vespa.hosted.provision.node.filter.StateFilter; import com.yahoo.vespa.hosted.provision.os.OsVersions; import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; import com.yahoo.vespa.hosted.provision.persistence.DnsNameResolver; +import com.yahoo.vespa.hosted.provision.persistence.JobControlFlags; import com.yahoo.vespa.hosted.provision.persistence.NameResolver; import com.yahoo.vespa.hosted.provision.provisioning.DockerImages; import com.yahoo.vespa.hosted.provision.provisioning.FirmwareChecks; @@ -114,14 +116,17 @@ public class NodeRepository extends AbstractComponent { NodeFlavors flavors, ProvisionServiceProvider provisionServiceProvider, Curator curator, - Zone zone) { + Zone zone, + FlagSource flagSource) { this(flavors, provisionServiceProvider.getHostResourcesCalculator(), curator, Clock.systemUTC(), zone, new DnsNameResolver(), - DockerImage.fromString(config.dockerImage()), config.useCuratorClientCache(), + DockerImage.fromString(config.dockerImage()), + flagSource, + config.useCuratorClientCache(), provisionServiceProvider.getHostProvisioner().isPresent(), zone.environment().isProduction() && provisionServiceProvider.getHostProvisioner().isEmpty() ? 1 : 0); } @@ -137,6 +142,7 @@ public class NodeRepository extends AbstractComponent { Zone zone, NameResolver nameResolver, DockerImage dockerImage, + FlagSource flagSource, boolean useCuratorClientCache, boolean canProvisionHosts, int spareCount) { @@ -150,7 +156,7 @@ public class NodeRepository extends AbstractComponent { this.infrastructureVersions = new InfrastructureVersions(db); this.firmwareChecks = new FirmwareChecks(db, clock); this.dockerImages = new DockerImages(db, dockerImage); - this.jobControl = new JobControl(db); + this.jobControl = new JobControl(new JobControlFlags(db, flagSource)); this.applications = new Applications(db); this.canProvisionHosts = canProvisionHosts; this.spareCount = spareCount; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index a8086400e26..5e2f0bd4761 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.provision.persistence; import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.component.Version; -import com.yahoo.concurrent.maintenance.JobControl; -import com.yahoo.concurrent.maintenance.StringSetSerializer; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.DockerImage; @@ -34,11 +32,9 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.TreeMap; import java.util.function.Function; import java.util.function.Predicate; @@ -59,7 +55,7 @@ import static java.util.stream.Collectors.toMap; * * @author bratseth */ -public class CuratorDatabaseClient implements JobControl.Db { +public class CuratorDatabaseClient { private static final Logger log = Logger.getLogger(CuratorDatabaseClient.class.getName()); @@ -77,7 +73,6 @@ public class CuratorDatabaseClient implements JobControl.Db { private static final Duration defaultLockTimeout = Duration.ofMinutes(2); private final NodeSerializer nodeSerializer; - private final StringSetSerializer stringSetSerializer = new StringSetSerializer(); private final CuratorDatabase db; private final Clock clock; private final Zone zone; @@ -437,37 +432,10 @@ public class CuratorDatabaseClient implements JobControl.Db { // Maintenance jobs ----------------------------------------------------------- - @Override public Lock lockMaintenanceJob(String jobName) { return db.lock(lockPath.append("maintenanceJobLocks").append(jobName), defaultLockTimeout); } - @Override - public Set<String> readInactiveJobs() { - try { - return read(inactiveJobsPath, stringSetSerializer::fromJson).orElseGet(HashSet::new); - } - catch (RuntimeException e) { - log.log(Level.WARNING, "Error reading inactive jobs, deleting inactive state"); - writeInactiveJobs(Collections.emptySet()); - return new HashSet<>(); - } - } - - @Override - public void writeInactiveJobs(Set<String> inactiveJobs) { - NestedTransaction transaction = new NestedTransaction(); - CuratorTransaction curatorTransaction = db.newCuratorTransactionIn(transaction); - curatorTransaction.add(CuratorOperations.setData(inactiveJobsPath.getAbsolute(), - stringSetSerializer.toJson(inactiveJobs))); - transaction.commit(); - } - - @Override - public Lock lockInactiveJobs() { - return db.lock(lockPath.append("inactiveJobsLock"), defaultLockTimeout); - } - // Infrastructure versions ----------------------------------------------------------- public Map<NodeType, Version> readInfrastructureVersions() { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/JobControlFlags.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/JobControlFlags.java new file mode 100644 index 00000000000..2a2a45186f9 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/JobControlFlags.java @@ -0,0 +1,36 @@ +package com.yahoo.vespa.hosted.provision.persistence; + +import com.yahoo.concurrent.maintenance.JobControlState; +import com.yahoo.transaction.Mutex; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.ListFlag; + +import java.util.Set; + +/** + * An implementation of {@link JobControlState} that uses a feature flag to control maintenance jobs. + * + * @author mpolden + */ +public class JobControlFlags implements JobControlState { + + private final CuratorDatabaseClient curator; + private final ListFlag<String> inactiveJobsFlag; + + public JobControlFlags(CuratorDatabaseClient curator, FlagSource flagSource) { + this.curator = curator; + this.inactiveJobsFlag = Flags.INACTIVE_MAINTENANCE_JOBS.bindTo(flagSource); + } + + @Override + public Set<String> readInactiveJobs() { + return Set.copyOf(inactiveJobsFlag.value()); + } + + @Override + public Mutex lockMaintenanceJob(String job) { + return curator.lockMaintenanceJob(job); + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java index 80136de6f03..5080dafe2a5 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiHandler.java @@ -185,7 +185,6 @@ public class NodesV2ApiHandler extends LoggingRequestHandler { int addedNodes = addNodes(request.getData()); return new MessageResponse("Added " + addedNodes + " nodes to the provisioned state"); } - if (path.matches("/nodes/v2/maintenance/inactive/{job}")) return setJobActive(path.get("job"), false); if (path.matches("/nodes/v2/maintenance/run/{job}")) return runJob(path.get("job")); if (path.matches("/nodes/v2/upgrade/firmware")) return requestFirmwareCheckResponse(); @@ -195,7 +194,6 @@ public class NodesV2ApiHandler extends LoggingRequestHandler { private HttpResponse handleDELETE(HttpRequest request) { Path path = new Path(request.getUri()); if (path.matches("/nodes/v2/node/{hostname}")) return deleteNode(path.get("hostname")); - if (path.matches("/nodes/v2/maintenance/inactive/{job}")) return setJobActive(path.get("job"), true); if (path.matches("/nodes/v2/upgrade/firmware")) return cancelFirmwareCheckResponse(); throw new NotFoundException("Nothing at path '" + request.getUri().getPath() + "'"); @@ -351,13 +349,6 @@ public class NodesV2ApiHandler extends LoggingRequestHandler { return false; } - private MessageResponse setJobActive(String jobName, boolean active) { - if ( ! nodeRepository.jobControl().jobs().contains(jobName)) - throw new NotFoundException("No job named '" + jobName + "'"); - nodeRepository.jobControl().setActive(jobName, active); - return new MessageResponse((active ? "Re-activated" : "Deactivated" ) + " job '" + jobName + "'"); - } - private MessageResponse setTargetVersions(HttpRequest request) { NodeType nodeType = NodeType.valueOf(lastElement(request.getUri().getPath()).toLowerCase()); Inspector inspector = toSlime(request.getData()).get(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java index bf56be4598c..f78560e3022 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockNodeRepository.java @@ -64,6 +64,7 @@ public class MockNodeRepository extends NodeRepository { Zone.defaultZone(), new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java index 034a9fb27d0..9b0500303d8 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTester.java @@ -9,6 +9,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.config.provisioning.FlavorsConfig; import com.yahoo.test.ManualClock; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.EmptyProvisionServiceProvider; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; @@ -40,6 +41,7 @@ public class NodeRepositoryTester { Zone.defaultZone(), new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java index cb3c7af797f..59e0dad9720 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java @@ -22,6 +22,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -63,6 +64,7 @@ public class CapacityCheckerTester { zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java index 7965293cf87..ed6f31984a5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/FailedExpirerTest.java @@ -258,6 +258,7 @@ public class FailedExpirerTest { zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-image"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java index 59540223797..39e873ee110 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceTester.java @@ -9,6 +9,7 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -40,6 +41,7 @@ public class MaintenanceTester { zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index 567cec7ff3e..20c9d24d1b6 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -16,6 +16,7 @@ import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -84,6 +85,7 @@ public class MetricsReporterTest { Zone.defaultZone(), new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); @@ -152,6 +154,7 @@ public class MetricsReporterTest { Zone.defaultZone(), new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java index a97675f5d5e..435dcdf9223 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailTester.java @@ -82,6 +82,7 @@ public class NodeFailTester { zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java index e342e3b7302..1ff00ff88a1 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/OperatorChangeApplicationMaintainerTest.java @@ -61,6 +61,7 @@ public class OperatorChangeApplicationMaintainerTest { zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java index 6d2b9a47e9f..c94d9022c3f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java @@ -67,6 +67,7 @@ public class PeriodicApplicationMaintainerTest { zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java index 705a038a79f..6ca154f5f17 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ReservationExpirerTest.java @@ -52,6 +52,7 @@ public class ReservationExpirerTest { Zone.defaultZone(), new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java index 6a85654a2dd..4e675d6a3cc 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirerTest.java @@ -71,6 +71,7 @@ public class RetiredExpirerTest { zone, new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 0); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java index ccb35e758bb..6cd206cd5b9 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -15,6 +15,7 @@ import com.yahoo.config.provision.Zone; import com.yahoo.test.ManualClock; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -245,6 +246,7 @@ public class SpareCapacityMaintainerTest { new Zone(Environment.prod, RegionName.from("us-east-3")), new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + new InMemoryFlagSource(), true, false, 1); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 204d1171919..b814bec683a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -104,6 +104,7 @@ public class ProvisioningTester { zone, nameResolver, DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), + flagSource, true, provisionServiceProvider.getHostProvisioner().isPresent(), spareCount); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java index 1e788e2c70e..b1ecd03aa13 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/NodesV2ApiTest.java @@ -253,28 +253,8 @@ public class NodesV2ApiTest { @Test public void maintenance_requests() throws Exception { - // POST deactivation of a maintenance job - assertResponse(new Request("http://localhost:8080/nodes/v2/maintenance/inactive/NodeFailer", - new byte[0], Request.Method.POST), - "{\"message\":\"Deactivated job 'NodeFailer'\"}"); // GET a list of all maintenance jobs assertFile(new Request("http://localhost:8080/nodes/v2/maintenance/"), "maintenance.json"); - - // DELETE deactivation of a maintenance job - assertResponse(new Request("http://localhost:8080/nodes/v2/maintenance/inactive/NodeFailer", - new byte[0], Request.Method.DELETE), - "{\"message\":\"Re-activated job 'NodeFailer'\"}"); - - // POST run of a maintenance job - assertResponse(new Request("http://localhost:8080/nodes/v2/maintenance/run/PeriodicApplicationMaintainer", - new byte[0], Request.Method.POST), - "{\"message\":\"Executed job 'PeriodicApplicationMaintainer'\"}"); - - // POST run of unknown maintenance job - tester.assertResponse(new Request("http://localhost:8080/nodes/v2/maintenance/run/foo", - new byte[0], Request.Method.POST), - 400, - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"No such job 'foo'\"}"); } @Test diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json index 6bb30d90218..804d8aa59d5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json @@ -52,13 +52,11 @@ "name": "RetiredExpirer" }, { - "name":"ScalingSuggestionsMaintainer" + "name": "ScalingSuggestionsMaintainer" }, { "name": "SpareCapacityMaintainer" } ], - "inactive": [ - "NodeFailer" - ] + "inactive": [] } diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobControl.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobControl.java index 896443117c9..583337203ab 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobControl.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobControl.java @@ -5,11 +5,12 @@ import com.yahoo.transaction.Mutex; import java.util.Collections; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentSkipListMap; /** - * Provides status and control over running maintenance jobs. + * Provides status over running maintenance jobs. * * This is multi-thread safe. * @@ -20,11 +21,11 @@ public class JobControl { /** This is not persisted as all nodes start all jobs */ private final Map<String, Maintainer> startedJobs = new ConcurrentSkipListMap<>(); - /** Used to store deactivation in a persistent shared database to make changes take effect on all nodes */ - private final Db db; + /** Used for managing shared persistent state, to make changes take effect on all nodes */ + private final JobControlState state; - public JobControl(Db db) { - this.db = db; + public JobControl(JobControlState state) { + this.state = Objects.requireNonNull(state); } /** Notifies this that a job was started */ @@ -39,25 +40,13 @@ public class JobControl { public Set<String> jobs() { return Collections.unmodifiableSet(startedJobs.keySet()); } /** Returns a snapshot containing the currently inactive jobs in this */ - public Set<String> inactiveJobs() { return db.readInactiveJobs(); } + public Set<String> inactiveJobs() { return state.readInactiveJobs(); } /** Returns true if this job is not currently deactivated */ public boolean isActive(String jobSimpleClassName) { return ! inactiveJobs().contains(jobSimpleClassName); } - /** Set a job active or inactive */ - public void setActive(String jobSimpleClassName, boolean active) { - try (var lock = db.lockInactiveJobs()) { - Set<String> inactiveJobs = db.readInactiveJobs(); - if (active) - inactiveJobs.remove(jobSimpleClassName); - else - inactiveJobs.add(jobSimpleClassName); - db.writeInactiveJobs(inactiveJobs); - } - } - /** Run given job (inactive or not) immediately */ public void run(String jobSimpleClassName) { var job = startedJobs.get(jobSimpleClassName); @@ -67,24 +56,7 @@ public class JobControl { /** Acquire lock for running given job */ public Mutex lockJob(String jobSimpleClassName) { - return db.lockMaintenanceJob(jobSimpleClassName); - } - - /** The database used for managing job state and synchronization */ - public interface Db { - - /** Returns the set of jobs that are temporarily inactive */ - Set<String> readInactiveJobs(); - - /** Make given jobs as inactive */ - void writeInactiveJobs(Set<String> inactiveJobs); - - /** Acquire lock for changing jobs */ - Mutex lockInactiveJobs(); - - /** Acquire lock for running given job */ - Mutex lockMaintenanceJob(String job); - + return state.lockMaintenanceJob(jobSimpleClassName); } } diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobControlState.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobControlState.java new file mode 100644 index 00000000000..f5bfeee8d0e --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/JobControlState.java @@ -0,0 +1,21 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.concurrent.maintenance; + +import com.yahoo.transaction.Mutex; + +import java.util.Set; + +/** + * Interface for managing job state and synchronization + * + * @author mpolden + */ +public interface JobControlState { + + /** Returns the set of jobs that are temporarily inactive */ + Set<String> readInactiveJobs(); + + /** Acquire lock for running given job */ + Mutex lockMaintenanceJob(String job); + +} diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/StringSetSerializer.java b/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/StringSetSerializer.java deleted file mode 100644 index 9612b5736d3..00000000000 --- a/vespajlib/src/main/java/com/yahoo/concurrent/maintenance/StringSetSerializer.java +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.concurrent.maintenance; - -import com.yahoo.slime.ArrayTraverser; -import com.yahoo.slime.Cursor; -import com.yahoo.slime.Inspector; -import com.yahoo.slime.Slime; -import com.yahoo.slime.SlimeUtils; - -import java.io.IOException; -import java.util.HashSet; -import java.util.Set; - -/** - * Serialization of a set of strings to/from Json bytes using Slime. - * - * The set is serialized as an array of string. - * - * @author bratseth - */ -public class StringSetSerializer { - - public byte[] toJson(Set<String> stringSet) { - try { - Slime slime = new Slime(); - Cursor array = slime.setArray(); - for (String element : stringSet) - array.addString(element); - return SlimeUtils.toJsonBytes(slime); - } catch (IOException e) { - throw new RuntimeException("Serialization of a string set failed", e); - } - - } - - public Set<String> fromJson(byte[] data) { - Inspector inspector = SlimeUtils.jsonToSlime(data).get(); - Set<String> stringSet = new HashSet<>(); - inspector.traverse((ArrayTraverser) (index, name) -> stringSet.add(name.asString())); - return stringSet; - } - -} diff --git a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java index 3600008b112..0640ab2835a 100644 --- a/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java +++ b/vespajlib/src/test/java/com/yahoo/concurrent/maintenance/JobControlTest.java @@ -21,7 +21,8 @@ public class JobControlTest { @Test public void testJobControl() { - JobControl jobControl = new JobControl(new MockDb()); + MockJobControlState state = new MockJobControlState(); + JobControl jobControl = new JobControl(state); MockMaintainer maintainer1 = new MockMaintainer(); MockMaintainer maintainer2 = new MockMaintainer(); @@ -39,19 +40,19 @@ public class JobControlTest { assertTrue(jobControl.isActive(job1)); assertTrue(jobControl.isActive(job2)); - jobControl.setActive(job1, false); + state.setActive(job1, false); assertFalse(jobControl.isActive(job1)); assertTrue(jobControl.isActive(job2)); - jobControl.setActive(job2, false); + state.setActive(job2, false); assertFalse(jobControl.isActive(job1)); assertFalse(jobControl.isActive(job2)); - jobControl.setActive(job1, true); + state.setActive(job1, true); assertTrue(jobControl.isActive(job1)); assertFalse(jobControl.isActive(job2)); - jobControl.setActive(job2, true); + state.setActive(job2, true); assertTrue(jobControl.isActive(job1)); assertTrue(jobControl.isActive(job2)); @@ -63,14 +64,15 @@ public class JobControlTest { assertEquals(1, maintainer2.maintenanceInvocations); // Running jobs on-demand ignores inactive flag - jobControl.setActive(job1, false); + state.setActive(job1, false); jobControl.run(job1); assertEquals(3, maintainer1.maintenanceInvocations); } @Test public void testJobControlMayDeactivateJobs() { - JobControl jobControl = new JobControl(new MockDb()); + MockJobControlState state = new MockJobControlState(); + JobControl jobControl = new JobControl(state); MockMaintainer mockMaintainer = new MockMaintainer(jobControl); assertTrue(jobControl.jobs().contains("MockMaintainer")); @@ -80,16 +82,16 @@ public class JobControlTest { mockMaintainer.run(); assertEquals(1, mockMaintainer.maintenanceInvocations); - jobControl.setActive("MockMaintainer", false); + state.setActive("MockMaintainer", false); mockMaintainer.run(); assertEquals(1, mockMaintainer.maintenanceInvocations); - jobControl.setActive("MockMaintainer", true); + state.setActive("MockMaintainer", true); mockMaintainer.run(); assertEquals(2, mockMaintainer.maintenanceInvocations); } - private static class MockDb implements JobControl.Db { + private static class MockJobControlState implements JobControlState { private final Set<String> inactiveJobs = new HashSet<>(); @@ -99,19 +101,16 @@ public class JobControlTest { } @Override - public void writeInactiveJobs(Set<String> inactiveJobs) { - this.inactiveJobs.clear(); - this.inactiveJobs.addAll(inactiveJobs); - } - - @Override - public Mutex lockInactiveJobs() { + public Mutex lockMaintenanceJob(String job) { return () -> {}; } - @Override - public Mutex lockMaintenanceJob(String job) { - return () -> {}; + public void setActive(String job, boolean active) { + if (active) { + inactiveJobs.remove(job); + } else { + inactiveJobs.add(job); + } } } @@ -125,7 +124,7 @@ public class JobControlTest { } private MockMaintainer() { - this(new JobControl(new MockDb())); + this(new JobControl(new MockJobControlState())); } @Override |