Update serialization format in a separate thread

5 files changed, 115 insertions, 25 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index a3a4e99c38d..af5b9198343 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -43,6 +43,7 @@ import com.yahoo.vespa.hosted.controller.application.JobList;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.application.JobStatus.JobRun;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
+import com.yahoo.vespa.hosted.controller.concurrent.Once;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.rotation.Rotation;
@@ -117,13 +118,17 @@ public class ApplicationController {
         this.rotationRepository = new RotationRepository(rotationsConfig, this, curator);
         this.deploymentTrigger = new DeploymentTrigger(controller, buildService, clock);
 
-        Instant start = clock.instant();
-        int count = 0;
-        for (Application application : curator.readApplications()) {
-            lockIfPresent(application.id(), this::store);
-            count++;
-        }
-        log.log(Level.INFO, String.format("Wrote %d applications in %s", count, Duration.between(start, clock.instant())));
+        // Update serialization format of all applications
+        Once.after(Duration.ofMinutes(1), () -> {
+            Instant start = clock.instant();
+            int count = 0;
+            for (Application application : curator.readApplications()) {
+                lockIfPresent(application.id(), this::store);
+                count++;
+            }
+            log.log(Level.INFO, String.format("Wrote %d applications in %s", count,
+                                              Duration.between(start, clock.instant())));
+        });
     }
 
     /** Returns the application with the given id, or null if it is not present */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
index 21db5c0b68f..1ae3e6a6577 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/TenantController.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsClient;
+import com.yahoo.vespa.hosted.controller.concurrent.Once;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
@@ -47,22 +48,30 @@ public class TenantController {
         this.curator = Objects.requireNonNull(curator, "curator must be non-null");
         this.athenzClientFactory = Objects.requireNonNull(athenzClientFactory, "athenzClientFactory must be non-null");
 
-        // Write all tenants to ensure persisted data uses latest serialization format
-        Instant start = controller.clock().instant();
-        int count = 0;
-        for (Tenant tenant : curator.readTenants()) {
-            try (Lock lock = lock(tenant.name())) {
-                if (tenant instanceof AthenzTenant) {
-                    curator.writeTenant((AthenzTenant) tenant);
-                } else if (tenant instanceof UserTenant) {
-                    curator.writeTenant((UserTenant) tenant);
-                } else {
-                    throw new IllegalArgumentException("Unknown tenant type: " + tenant.getClass().getSimpleName());
+        // Update serialization format of all tenants
+        Once.after(Duration.ofMinutes(1), () -> {
+            Instant start = controller.clock().instant();
+            int count = 0;
+            for (TenantName name : curator.readTenantNames()) {
+                try (Lock lock = lock(name)) {
+                    // Get while holding lock so that we know we're operating on a current version
+                    Optional<Tenant> optionalTenant = tenant(name);
+                    if (!optionalTenant.isPresent()) continue; // Deleted while updating, skip
+
+                    Tenant tenant = optionalTenant.get();
+                    if (tenant instanceof AthenzTenant) {
+                        curator.writeTenant((AthenzTenant) tenant);
+                    } else if (tenant instanceof UserTenant) {
+                        curator.writeTenant((UserTenant) tenant);
+                    } else {
+                        throw new IllegalArgumentException("Unknown tenant type: " + tenant.getClass().getSimpleName());
+                    }
                 }
+                count++;
             }
-            count++;
-        }
-        log.log(Level.INFO, String.format("Wrote %d tenants in %s", count, Duration.between(start, controller.clock().instant())));
+            log.log(Level.INFO, String.format("Wrote %d tenants in %s", count,
+                                              Duration.between(start, controller.clock().instant())));
+        });
     }
 
     /** Returns a list of all known tenants sorted by name */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/concurrent/Once.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/concurrent/Once.java
new file mode 100644
index 00000000000..81ddd8d2d70
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/concurrent/Once.java
@@ -0,0 +1,46 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.concurrent;
+
+import java.time.Duration;
+import java.util.Objects;
+import java.util.Timer;
+import java.util.TimerTask;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Execute a runnable exactly once in a background thread.
+ *
+ * @author mpolden
+ */
+public class Once extends TimerTask {
+
+    private static final Logger log = Logger.getLogger(Once.class.getName());
+
+    private final Runnable runnable;
+    private final Timer timer = new Timer(true);
+
+    // private to avoid exposing run method
+    private Once(Runnable runnable, Duration delay) {
+        this.runnable = Objects.requireNonNull(runnable, "runnable must be non-null");
+        Objects.requireNonNull(delay, "delay must be non-null");
+        timer.schedule(this, delay.toMillis());
+    }
+
+    /** Execute runnable after given delay */
+    public static void after(Duration delay, Runnable runnable) {
+        new Once(runnable, delay);
+    }
+
+    @Override
+    public void run() {
+        try {
+            runnable.run();
+        } catch (Throwable t) {
+            log.log(Level.WARNING, "Task '" + runnable + "' failed", t);
+        } finally {
+            timer.cancel();
+        }
+    }
+
+}
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 e117592d608..0d8ea8d2537 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
@@ -302,12 +302,17 @@ public class CuratorDb {
     }
 
     public List<Tenant> readTenants() {
+        return readTenantNames().stream()
+                                .map(this::readTenant)
+                                .filter(Optional::isPresent)
+                                .map(Optional::get)
+                                .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
+    }
+
+    public List<TenantName> readTenantNames() {
         return curator.getChildren(tenantRoot).stream()
                       .map(TenantName::from)
-                      .map(this::readTenant)
-                      .filter(Optional::isPresent)
-                      .map(Optional::get)
-                      .collect(collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
+                      .collect(Collectors.toList());
     }
 
     public void removeTenant(TenantName name) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/concurrent/OnceTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/concurrent/OnceTest.java
new file mode 100644
index 00000000000..e11fdcba7c6
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/concurrent/OnceTest.java
@@ -0,0 +1,25 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.concurrent;
+
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author mpolden
+ */
+public class OnceTest {
+
+    @Test(timeout = 60_000)
+    public void test_run() throws Exception {
+        CountDownLatch latch = new CountDownLatch(1);
+        Once.after(Duration.ZERO, latch::countDown);
+
+        assertTrue(latch.await(30, TimeUnit.SECONDS));
+    }
+
+}