Merge pull request #11604 from vespa-engine/mpolden/application-cache

Cache deserialized applications

3 files changed, 29 insertions, 18 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
index dffc1dfe88e..3758fb8476d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
@@ -1,6 +1,10 @@
 // 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.persistence;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.hash.Hashing;
+import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentInstanceSpec;
 import com.yahoo.config.application.api.DeploymentSpec;
@@ -15,6 +19,7 @@ import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.ObjectTraverser;
 import com.yahoo.slime.Slime;
+import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
@@ -49,6 +54,7 @@ import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
 import java.util.Set;
+import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
 
 /**
@@ -56,6 +62,7 @@ import java.util.stream.Collectors;
  * This class is multithread safe.
  *
  * @author jonmv
+ * @author mpolden
  */
 public class ApplicationSerializer {
 
@@ -87,6 +94,7 @@ public class ApplicationSerializer {
     private static final String pemDeployKeysField = "pemDeployKeys";
     private static final String assignedRotationClusterField = "clusterId";
     private static final String assignedRotationRotationField = "rotationId";
+    private static final String versionField = "version";
 
     // Instance fields
     private static final String instanceNameField = "instanceName";
@@ -121,15 +129,6 @@ public class ApplicationSerializer {
     private static final String jobTypeField = "jobType";
     private static final String pausedUntilField = "pausedUntil";
 
-    // JobRun fields
-    private static final String jobRunIdField = "id";
-    private static final String versionField = "version";
-    private static final String revisionField = "revision";
-    private static final String sourceVersionField = "sourceVersion";
-    private static final String sourceApplicationField = "sourceRevision";
-    private static final String reasonField = "reason";
-    private static final String atField = "at";
-
     // ClusterInfo fields
     private static final String clusterInfoField = "clusterInfo";
     private static final String clusterInfoFlavorField = "flavor";
@@ -157,6 +156,13 @@ public class ApplicationSerializer {
     private static final String rotationStateField = "state";
     private static final String statusField = "status";
 
+    // A cache of deserialized applications.
+    //
+    // Deserializing an application from slime is expensive, particularly XML fields, such as DeploymentSpec and
+    // ValidationOverrides. Applications that have already been deserialized are returned from this cache instead of
+    // being deserialized again.
+    private final Cache<Long, Application> cache = CacheBuilder.newBuilder().maximumSize(1000).build();
+
     // ------------------ Serialization
 
     public Slime toSlime(Application application) {
@@ -317,7 +323,16 @@ public class ApplicationSerializer {
 
     // ------------------ Deserialization
 
-    public Application fromSlime(Slime slime) {
+    public Application fromSlime(byte[] data) {
+        var key = Hashing.sipHash24().hashBytes(data).asLong();
+        try {
+            return cache.get(key, () -> fromSlime(SlimeUtils.jsonToSlime(data)));
+        } catch (ExecutionException e) {
+            throw new UncheckedExecutionException(e);
+        }
+    }
+
+    private Application fromSlime(Slime slime) {
         Inspector root = slime.get();
 
         TenantAndApplicationId id = TenantAndApplicationId.fromSerialized(root.field(idField).asString());
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 637da842d2f..0f2fb4be27a 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
@@ -42,7 +42,6 @@ import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Optional;
 import java.util.Set;
-import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeoutException;
@@ -52,7 +51,6 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.LongStream;
-import java.util.stream.Stream;
 
 import static java.util.Comparator.comparing;
 import static java.util.stream.Collectors.collectingAndThen;
@@ -341,7 +339,7 @@ public class CuratorDb {
     }
 
     public Optional<Application> readApplication(TenantAndApplicationId application) {
-        return readSlime(applicationPath(application)).map(applicationSerializer::fromSlime);
+        return read(applicationPath(application), applicationSerializer::fromSlime);
     }
 
     public List<Application> readApplications() {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
index 4b0f5dd9a17..4f396321544 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
@@ -45,8 +45,6 @@ import java.util.OptionalInt;
 import java.util.OptionalLong;
 import java.util.Set;
 
-import static com.yahoo.config.provision.SystemName.main;
-import static java.util.Optional.empty;
 import static org.junit.Assert.assertEquals;
 
 /**
@@ -70,7 +68,7 @@ public class ApplicationSerializerTest {
 
 
     @Test
-    public void testSerialization() {
+    public void testSerialization() throws Exception {
         DeploymentSpec deploymentSpec = DeploymentSpec.fromXml("<deployment version='1.0'>\n" +
                                                                "   <staging/>\n" +
                                                                "   <instance id=\"i1\">\n" +
@@ -135,7 +133,7 @@ public class ApplicationSerializerTest {
                                                Optional.of(applicationVersion1),
                                                instances);
 
-        Application serialized = APPLICATION_SERIALIZER.fromSlime(APPLICATION_SERIALIZER.toSlime(original));
+        Application serialized = APPLICATION_SERIALIZER.fromSlime(SlimeUtils.toJsonBytes(APPLICATION_SERIALIZER.toSlime(original)));
 
         assertEquals(original.id(), serialized.id());
         assertEquals(original.createdAt(), serialized.createdAt());
@@ -217,7 +215,7 @@ public class ApplicationSerializerTest {
     @Test
     public void testCompleteApplicationDeserialization() throws Exception {
         byte[] applicationJson = Files.readAllBytes(testData.resolve("complete-application.json"));
-        APPLICATION_SERIALIZER.fromSlime(SlimeUtils.jsonToSlime(applicationJson));
+        APPLICATION_SERIALIZER.fromSlime(applicationJson);
         // ok if no error
     }