Cache snapshotting of ServiceModel

5 files changed, 151 insertions, 16 deletions

diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java
index c6fe584c30a..80be6470686 100644
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/ServiceMonitorImpl.java
@@ -9,6 +9,7 @@ import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.Timer;
 import com.yahoo.vespa.applicationmodel.ApplicationInstance;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
+import com.yahoo.vespa.service.monitor.internal.ServiceModelCache;
 import com.yahoo.vespa.service.monitor.internal.ServiceMonitorMetrics;
 
 import java.util.Collections;
@@ -23,7 +24,7 @@ public class ServiceMonitorImpl implements ServiceMonitor {
     private final Zone zone;
     private final List<String> configServerHosts;
     private final SlobrokMonitorManager slobrokMonitorManager = new SlobrokMonitorManager();
-    private final SuperModelListenerImpl superModelListener;
+    private final ServiceModelCache serviceModelCache;
 
     @Inject
     public ServiceMonitorImpl(SuperModelProvider superModelProvider,
@@ -33,11 +34,17 @@ public class ServiceMonitorImpl implements ServiceMonitor {
         this.zone = superModelProvider.getZone();
         this.configServerHosts = toConfigServerList(configserverConfig);
         ServiceMonitorMetrics metrics = new ServiceMonitorMetrics(metric, timer);
-        this.superModelListener = new SuperModelListenerImpl(
+
+        SuperModelListenerImpl superModelListener = new SuperModelListenerImpl(
                 slobrokMonitorManager,
                 metrics,
-                new ModelGenerator());
+                new ModelGenerator(),
+                zone,
+                configServerHosts);
         superModelListener.start(superModelProvider);
+        serviceModelCache = new ServiceModelCache(
+                () -> superModelListener.get(),
+                timer);
     }
 
     private List<String> toConfigServerList(ConfigserverConfig configserverConfig) {
@@ -53,10 +60,6 @@ public class ServiceMonitorImpl implements ServiceMonitor {
     @Override
     public Map<ApplicationInstanceReference,
             ApplicationInstance<ServiceMonitorStatus>> queryStatusOfAllApplicationInstances() {
-        // If we ever need to optimize this method, then consider reusing ServiceModel snapshots
-        // for up to X ms.
-        ServiceModel serviceModel =
-                superModelListener.createServiceModelSnapshot(zone, configServerHosts);
-        return serviceModel.getAllApplicationInstances();
+        return serviceModelCache.get().getAllApplicationInstances();
     }
 }
diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/SuperModelListenerImpl.java b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/SuperModelListenerImpl.java
index 56b42a80a79..077cb75ac2c 100644
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/SuperModelListenerImpl.java
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/SuperModelListenerImpl.java
@@ -11,13 +11,16 @@ import com.yahoo.vespa.service.monitor.internal.LatencyMeasurement;
 import com.yahoo.vespa.service.monitor.internal.ServiceMonitorMetrics;
 
 import java.util.List;
+import java.util.function.Supplier;
 import java.util.logging.Logger;
 
-public class SuperModelListenerImpl implements SuperModelListener {
+public class SuperModelListenerImpl implements SuperModelListener, Supplier<ServiceModel> {
     private static final Logger logger = Logger.getLogger(SuperModelListenerImpl.class.getName());
 
     private final ServiceMonitorMetrics metrics;
     private final ModelGenerator modelGenerator;
+    private final Zone zone;
+    private final List<String> configServerHosts;
 
     // superModel and slobrokMonitorManager are always updated together
     // and atomically using this monitor.
@@ -27,10 +30,14 @@ public class SuperModelListenerImpl implements SuperModelListener {
 
     SuperModelListenerImpl(SlobrokMonitorManager slobrokMonitorManager,
                            ServiceMonitorMetrics metrics,
-                           ModelGenerator modelGenerator) {
+                           ModelGenerator modelGenerator,
+                           Zone zone,
+                           List<String> configServerHosts) {
         this.slobrokMonitorManager = slobrokMonitorManager;
         this.metrics = metrics;
         this.modelGenerator = modelGenerator;
+        this.zone = zone;
+        this.configServerHosts = configServerHosts;
     }
 
     void start(SuperModelProvider superModelProvider) {
@@ -61,7 +68,8 @@ public class SuperModelListenerImpl implements SuperModelListener {
         }
     }
 
-    ServiceModel createServiceModelSnapshot(Zone zone, List<String> configServerHostnames) {
+    @Override
+    public ServiceModel get() {
         try (LatencyMeasurement measurement = metrics.startServiceModelSnapshotLatencyMeasurement()) {
             // Reference 'measurement' in a dummy statement, otherwise the compiler
             // complains about "auto-closeable resource is never referenced in body of
@@ -72,7 +80,7 @@ public class SuperModelListenerImpl implements SuperModelListener {
             return modelGenerator.toServiceModel(
                     superModel,
                     zone,
-                    configServerHostnames,
+                    configServerHosts,
                     slobrokMonitorManager);
         }
     }
diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/internal/ServiceModelCache.java b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/internal/ServiceModelCache.java
new file mode 100644
index 00000000000..89a961e1037
--- /dev/null
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/internal/ServiceModelCache.java
@@ -0,0 +1,62 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+package com.yahoo.vespa.service.monitor.internal;
+
+import com.yahoo.jdisc.Timer;
+import com.yahoo.vespa.service.monitor.ServiceModel;
+
+import java.util.function.Supplier;
+
+public class ServiceModelCache implements Supplier<ServiceModel> {
+    public static final long EXPIRY_MILLIS = 10000;
+
+    private final Supplier<ServiceModel> expensiveSupplier;
+    private final Timer timer;
+
+    private volatile ServiceModel snapshot;
+    private boolean updatePossiblyInProgress = false;
+
+    private final Object updateMonitor = new Object();
+    private long snapshotMillis;
+
+    public ServiceModelCache(Supplier<ServiceModel> expensiveSupplier, Timer timer) {
+        this.expensiveSupplier = expensiveSupplier;
+        this.timer = timer;
+    }
+
+    @Override
+    public ServiceModel get() {
+        if (snapshot == null) {
+            synchronized (updateMonitor) {
+                if (snapshot == null) {
+                    takeSnapshot();
+                }
+            }
+        } else if (expired()) {
+            synchronized (updateMonitor) {
+                if (updatePossiblyInProgress) {
+                    return snapshot;
+                }
+
+                updatePossiblyInProgress = true;
+            }
+
+            takeSnapshot();
+
+            synchronized (updateMonitor) {
+                updatePossiblyInProgress = false;
+            }
+        }
+
+        return snapshot;
+    }
+
+    private void takeSnapshot() {
+        snapshot = expensiveSupplier.get();
+        snapshotMillis = timer.currentTimeMillis();
+    }
+
+    private boolean expired() {
+        return timer.currentTimeMillis() - snapshotMillis >= EXPIRY_MILLIS;
+    }
+}
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/SuperModelListenerImplTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/SuperModelListenerImplTest.java
index 0451785f34d..e5608e92255 100644
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/SuperModelListenerImplTest.java
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/SuperModelListenerImplTest.java
@@ -23,10 +23,14 @@ public class SuperModelListenerImplTest {
         SlobrokMonitorManager slobrokMonitorManager = mock(SlobrokMonitorManager.class);
         ServiceMonitorMetrics metrics = mock(ServiceMonitorMetrics.class);
         ModelGenerator modelGenerator = mock(ModelGenerator.class);
+        Zone zone = mock(Zone.class);
+        List<String> configServers = new ArrayList<>();
         SuperModelListenerImpl listener = new SuperModelListenerImpl(
                 slobrokMonitorManager,
                 metrics,
-                modelGenerator);
+                modelGenerator,
+                zone,
+                configServers);
 
         SuperModelProvider superModelProvider = mock(SuperModelProvider.class);
         SuperModel superModel = mock(SuperModel.class);
@@ -42,9 +46,7 @@ public class SuperModelListenerImplTest {
         verify(slobrokMonitorManager).applicationActivated(superModel, application1);
         verify(slobrokMonitorManager).applicationActivated(superModel, application2);
 
-        Zone zone = mock(Zone.class);
-        List<String> configServers = new ArrayList<>();
-        ServiceModel serviceModel = listener.createServiceModelSnapshot(zone, configServers);
+        ServiceModel serviceModel = listener.get();
         verify(modelGenerator).toServiceModel(superModel, zone, configServers, slobrokMonitorManager);
     }
 }
\ No newline at end of file
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/internal/ServiceModelCacheTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/internal/ServiceModelCacheTest.java
new file mode 100644
index 00000000000..2ba6ce0deb8
--- /dev/null
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/internal/ServiceModelCacheTest.java
@@ -0,0 +1,60 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+package com.yahoo.vespa.service.monitor.internal;
+
+import com.yahoo.jdisc.Timer;
+import com.yahoo.vespa.service.monitor.ServiceModel;
+import org.junit.Test;
+
+import java.util.function.Supplier;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class ServiceModelCacheTest {
+    @SuppressWarnings("unchecked")
+    private final Supplier<ServiceModel> rawSupplier = mock(Supplier.class);
+    private final Timer timer = mock(Timer.class);
+    private final ServiceModelCache cache = new ServiceModelCache(rawSupplier, timer);
+
+    @Test
+    public void sanityCheck() {
+        ServiceModel serviceModel = mock(ServiceModel.class);
+        when(rawSupplier.get()).thenReturn(serviceModel);
+
+        long timeMillis = 0;
+        when(timer.currentTimeMillis()).thenReturn(timeMillis);
+
+        // Will always populate cache the first time
+        ServiceModel actualServiceModel = cache.get();
+        assertTrue(actualServiceModel == serviceModel);
+        verify(rawSupplier, times(1)).get();
+
+        // Cache hit
+        timeMillis += ServiceModelCache.EXPIRY_MILLIS / 2;
+        when(timer.currentTimeMillis()).thenReturn(timeMillis);
+        actualServiceModel = cache.get();
+        assertTrue(actualServiceModel == serviceModel);
+
+        // Cache expired
+        timeMillis += ServiceModelCache.EXPIRY_MILLIS + 1;
+        when(timer.currentTimeMillis()).thenReturn(timeMillis);
+
+        ServiceModel serviceModel2 = mock(ServiceModel.class);
+        when(rawSupplier.get()).thenReturn(serviceModel2);
+
+        actualServiceModel = cache.get();
+        assertTrue(actualServiceModel == serviceModel2);
+        // '2' because it's cumulative with '1' from the first times(1).
+        verify(rawSupplier, times(2)).get();
+
+        // Cache hit #2
+        timeMillis += 1;
+        when(timer.currentTimeMillis()).thenReturn(timeMillis);
+        actualServiceModel = cache.get();
+        assertTrue(actualServiceModel == serviceModel2);
+    }
+}
\ No newline at end of file