Merge pull request #14424 from vespa-engine/hmusum/do-not-maintain-when-bootstrapping-is-in-progress

Wait with deployments until bootstrapping has finished

6 files changed, 40 insertions, 18 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<Instant> 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..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
@@ -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<Provisioner> hostProvisioner;
     private final Optional<InfraDeployer> 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);
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<ApplicationId> 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<Node> nodesNeedingMaintenance() {
         return nodeRepository().getNodes(Node.State.active);
     }
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<Deployment> 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 {