From c12dfa2fef99ad6bf76cf7560002b5f821bcab23 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 16 Sep 2020 14:06:23 +0200 Subject: Wait with deployments until bootstrapping has finished --- .../src/main/java/com/yahoo/config/provision/Deployer.java | 3 +++ .../yahoo/vespa/config/server/ApplicationRepository.java | 14 ++++++++++++++ .../yahoo/vespa/config/server/ConfigServerBootstrap.java | 4 +--- .../maintenance/PeriodicApplicationMaintainer.java | 12 +----------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java index 2e53d4384a4..9cfe3fdd1cc 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployer.java @@ -65,4 +65,7 @@ public interface Deployer { /** Returns the time the current local active session was created, or empty if there is no local active session */ Optional lastDeployTime(ApplicationId application); + /** Whether the deployer is bootstrapping, some users of the deployer will want to hold off with deployments in that case. */ + default boolean bootstrapping() { return false; }; + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 824d6701233..59b9c549319 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -87,6 +87,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -112,6 +113,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private static final Logger log = Logger.getLogger(ApplicationRepository.class.getName()); + private final AtomicBoolean bootstrapping = new AtomicBoolean(true); + private final TenantRepository tenantRepository; private final Optional hostProvisioner; private final Optional infraDeployer; @@ -268,6 +271,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye // ---------------- Deploying ---------------------------------------------------------------- + @Override + public boolean bootstrapping() { + return bootstrapping.get(); + } + + public void bootstrappingDone() { + bootstrapping.set(false); + } + public PrepareResult prepare(Tenant tenant, long sessionId, PrepareParams prepareParams, Instant now) { validateThatLocalSessionIsNotActive(tenant, sessionId); LocalSession session = getLocalSession(tenant, sessionId); @@ -367,6 +379,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye SessionRepository sessionRepository = tenant.getSessionRepository(); LocalSession newSession = sessionRepository.createSessionFromExisting(activeSession, logger, true, timeoutBudget); sessionRepository.addLocalSession(newSession); + sessionRepository.createRemoteSession(newSession.getSessionId()); return Optional.of(Deployment.unprepared(newSession, this, hostProvisioner, tenant, timeout, clock, false /* don't validate as this is already deployed */, bootstrap)); @@ -815,6 +828,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye timeoutBudget, activeSessionId); tenant.getSessionRepository().addLocalSession(session); + tenant.getSessionRepository().createRemoteSession(session.getSessionId()); return session.getSessionId(); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java index 749561d5fdb..609ff4473c6 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ConfigServerBootstrap.java @@ -10,11 +10,8 @@ import com.yahoo.config.provision.Deployment; import com.yahoo.config.provision.TransientException; import com.yahoo.container.handler.VipStatus; import com.yahoo.container.jdisc.state.StateMonitor; -import com.yahoo.vespa.config.server.maintenance.FileDistributionMaintainer; import com.yahoo.vespa.config.server.rpc.RpcServer; import com.yahoo.vespa.config.server.version.VersionState; -import com.yahoo.vespa.curator.Curator; -import com.yahoo.vespa.flags.FlagSource; import com.yahoo.yolean.Exceptions; import java.time.Duration; @@ -166,6 +163,7 @@ public class ConfigServerBootstrap extends AbstractComponent implements Runnable return; // Status will not be set to 'up' since we return here } } + applicationRepository.bootstrappingDone(); startRpcServer(); up(); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java index 3cb0f935888..e3128dfc8e9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java @@ -7,7 +7,6 @@ import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; -import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.LinkedHashSet; @@ -27,15 +26,11 @@ import java.util.stream.Collectors; public class PeriodicApplicationMaintainer extends ApplicationMaintainer { private final Duration minTimeBetweenRedeployments; - private final Clock clock; - private final Instant start; PeriodicApplicationMaintainer(Deployer deployer, Metric metric, NodeRepository nodeRepository, Duration interval, Duration minTimeBetweenRedeployments) { super(deployer, metric, nodeRepository, interval); this.minTimeBetweenRedeployments = minTimeBetweenRedeployments; - this.clock = nodeRepository.clock(); - this.start = clock.instant(); } @Override @@ -51,7 +46,7 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer { // Returns the applications that need to be redeployed by this config server at this point in time. @Override protected Set applicationsNeedingMaintenance() { - if (waitInitially()) return Set.of(); + if (deployer().bootstrapping()) return Set.of(); // Collect all deployment times before sorting as deployments may happen while we build the set, breaking // the comparable contract. Stale times are fine as the time is rechecked in ApplicationMaintainer#deployWithLock @@ -75,11 +70,6 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer { return true; } - // TODO: Do not start deploying until some time has gone (ideally only until bootstrap of config server is finished) - private boolean waitInitially() { - return clock.instant().isBefore(start.plus(minTimeBetweenRedeployments)); - } - protected List nodesNeedingMaintenance() { return nodeRepository().getNodes(Node.State.active); } -- cgit v1.2.3 From 14ff7194bf46c5d56faabcf0a26d7606b3d0c927 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 16 Sep 2020 14:58:24 +0200 Subject: Test with bootstrapping flag set --- .../vespa/hosted/provision/testutils/MockDeployer.java | 9 ++++++++- .../maintenance/PeriodicApplicationMaintainerTest.java | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java index ae3d6ebf815..06ece5b3767 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java @@ -12,7 +12,6 @@ import com.yahoo.config.provision.HostSpec; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; -import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner; import java.time.Clock; @@ -44,6 +43,7 @@ public class MockDeployer implements Deployer { private final ReentrantLock lock = new ReentrantLock(); private boolean failActivate = false; + private boolean bootstrapping = true; /** Create a mock deployer which returns empty on every deploy request. */ @Inject @@ -83,6 +83,8 @@ public class MockDeployer implements Deployer { public void setFailActivate(boolean failActivate) { this.failActivate = failActivate; } + public void setBootstrapping(boolean bootstrapping) { this.bootstrapping = bootstrapping; } + @Override public Optional deployFromLocalActive(ApplicationId id, boolean bootstrap) { return deployFromLocalActive(id, Duration.ofSeconds(60)); @@ -117,6 +119,11 @@ public class MockDeployer implements Deployer { return Optional.ofNullable(lastDeployTimes.get(application)); } + @Override + public boolean bootstrapping() { + return bootstrapping; + } + public void removeApplication(ApplicationId applicationId) { new MockDeployment(provisioner, new ApplicationContext(applicationId, List.of())).activate(); 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 099282205fe..ec830a7dc31 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 @@ -87,8 +87,9 @@ public class PeriodicApplicationMaintainerTest { // Create applications fixture.activate(); - // Exhaust initial wait period + // Exhaust initial wait period and set bootstrapping to be done clock.advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1))); + fixture.setBootstrapping(false); // Fail and park some nodes nodeRepository.fail(nodeRepository.getNodes(fixture.app1).get(3).hostname(), Agent.system, "Failing to unit test"); @@ -161,9 +162,15 @@ public class PeriodicApplicationMaintainerTest { // Exhaust initial wait period clock.advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1))); - // First deployment of applications + // Will not do any deployments, as bootstrapping is still in progress + fixture.runApplicationMaintainer(); + assertEquals("No deployment expected", 2, fixture.deployer.redeployments); + + // First deployment of applications will happen now, as bootstrapping is done + fixture.setBootstrapping(false); fixture.runApplicationMaintainer(); assertEquals("No deployment expected", 4, fixture.deployer.redeployments); + Instant firstDeployTime = clock.instant(); assertEquals(firstDeployTime, fixture.deployer.lastDeployTime(fixture.app1).get()); assertEquals(firstDeployTime, fixture.deployer.lastDeployTime(fixture.app2).get()); @@ -186,8 +193,9 @@ public class PeriodicApplicationMaintainerTest { public void queues_all_eligible_applications_for_deployment() throws Exception { fixture.activate(); - // Exhaust initial wait period + // Exhaust initial wait period and set bootstrapping to be done clock.advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1))); + fixture.setBootstrapping(false); // Lock deployer to simulate slow deployments fixture.deployer.lock().lockInterruptibly(); @@ -298,6 +306,10 @@ public class PeriodicApplicationMaintainerTest { return NodeList.copyOf(nodeRepository.getNodes(NodeType.tenant, states)); } + void setBootstrapping(boolean bootstrapping) { + deployer.setBootstrapping(bootstrapping); + } + } private static class TestablePeriodicApplicationMaintainer extends PeriodicApplicationMaintainer { -- cgit v1.2.3 From acc3d54710c128616e1b50bb50f2d93f5fb45f38 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 16 Sep 2020 15:29:03 +0200 Subject: Revert unintended changes --- .../main/java/com/yahoo/vespa/config/server/ApplicationRepository.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 59b9c549319..3564a6e6da7 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -379,7 +379,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye SessionRepository sessionRepository = tenant.getSessionRepository(); LocalSession newSession = sessionRepository.createSessionFromExisting(activeSession, logger, true, timeoutBudget); sessionRepository.addLocalSession(newSession); - sessionRepository.createRemoteSession(newSession.getSessionId()); return Optional.of(Deployment.unprepared(newSession, this, hostProvisioner, tenant, timeout, clock, false /* don't validate as this is already deployed */, bootstrap)); @@ -828,7 +827,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye timeoutBudget, activeSessionId); tenant.getSessionRepository().addLocalSession(session); - tenant.getSessionRepository().createRemoteSession(session.getSessionId()); return session.getSessionId(); } -- cgit v1.2.3