Remove service model cache

5 files changed, 59 insertions, 279 deletions

diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelCache.java b/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelCache.java
deleted file mode 100644
index cc0304e996a..00000000000
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelCache.java
+++ /dev/null
@@ -1,74 +0,0 @@
-// 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.model;
-
-import com.yahoo.jdisc.Timer;
-import com.yahoo.vespa.service.monitor.ServiceHostListener;
-import com.yahoo.vespa.service.monitor.ServiceModel;
-import com.yahoo.vespa.service.monitor.ServiceMonitor;
-
-/**
- * Adds caching of a supplier of ServiceModel.
- *
- * @author hakonhall
- */
-public class ServiceModelCache implements ServiceMonitor {
-    public static final long EXPIRY_MILLIS = 10000;
-
-    private final ServiceMonitor expensiveServiceMonitor;
-    private final Timer timer;
-
-    private volatile ServiceModel snapshot;
-    private boolean updatePossiblyInProgress = false;
-
-    private final Object updateMonitor = new Object();
-    private long snapshotMillis;
-
-    public ServiceModelCache(ServiceMonitor expensiveServiceMonitor, Timer timer) {
-        this.expensiveServiceMonitor = expensiveServiceMonitor;
-        this.timer = timer;
-    }
-
-    @Override
-    public ServiceModel getServiceModelSnapshot() {
-        if (snapshot == null) {
-            synchronized (updateMonitor) {
-                if (snapshot == null) {
-                    takeSnapshot();
-                }
-            }
-        } else if (expired()) {
-            synchronized (updateMonitor) {
-                if (updatePossiblyInProgress) {
-                    return snapshot;
-                }
-
-                updatePossiblyInProgress = true;
-            }
-
-            try {
-                takeSnapshot();
-            } finally {
-                synchronized (updateMonitor) {
-                    updatePossiblyInProgress = false;
-                }
-            }
-        }
-
-        return snapshot;
-    }
-
-    @Override
-    public void registerListener(ServiceHostListener listener) {
-        expensiveServiceMonitor.registerListener(listener);
-    }
-
-    private void takeSnapshot() {
-        snapshot = expensiveServiceMonitor.getServiceModelSnapshot();
-        snapshotMillis = timer.currentTimeMillis();
-    }
-
-    private boolean expired() {
-        return timer.currentTimeMillis() - snapshotMillis >= EXPIRY_MILLIS;
-    }
-}
diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelProvider.java b/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelProvider.java
deleted file mode 100644
index d4a46bd59a9..00000000000
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelProvider.java
+++ /dev/null
@@ -1,121 +0,0 @@
-// 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.model;
-
-import com.yahoo.config.model.api.ApplicationInfo;
-import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.Zone;
-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.applicationmodel.HostName;
-import com.yahoo.vespa.applicationmodel.ServiceInstance;
-import com.yahoo.vespa.service.duper.DuperModelManager;
-import com.yahoo.vespa.service.manager.MonitorManager;
-import com.yahoo.vespa.service.manager.UnionMonitorManager;
-import com.yahoo.vespa.service.monitor.ServiceHostListener;
-import com.yahoo.vespa.service.monitor.ServiceModel;
-import com.yahoo.vespa.service.monitor.ServiceMonitor;
-import com.yahoo.vespa.service.monitor.ServiceStatusProvider;
-
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
-
-/**
- * An uncached supplier of ServiceModel based on the DuperModel and MonitorManager.
- *
- * @author hakonhall
- */
-public class ServiceModelProvider implements ServiceMonitor {
-    private final ServiceMonitorMetrics metrics;
-    private final DuperModelManager duperModelManager;
-    private final ModelGenerator modelGenerator;
-    private final ServiceStatusProvider serviceStatusProvider;
-
-    public ServiceModelProvider(DuperModelManager duperModelManager,
-                                UnionMonitorManager monitorManager,
-                                Metric metric,
-                                Timer timer,
-                                Zone zone) {
-        this(monitorManager,
-                new ServiceMonitorMetrics(metric, timer),
-                duperModelManager,
-                new ModelGenerator(zone)
-        );
-    }
-
-    ServiceModelProvider(MonitorManager monitorManager,
-                         ServiceMonitorMetrics metrics,
-                         DuperModelManager duperModelManager,
-                         ModelGenerator modelGenerator) {
-        this.serviceStatusProvider = monitorManager;
-        this.metrics = metrics;
-        this.duperModelManager = duperModelManager;
-        this.modelGenerator = modelGenerator;
-
-        duperModelManager.registerListener(monitorManager);
-    }
-
-    @Override
-    public ServiceModel getServiceModelSnapshot() {
-        try (LatencyMeasurement measurement = metrics.startServiceModelSnapshotLatencyMeasurement()) {
-            return modelGenerator.toServiceModel(duperModelManager.getApplicationInfos(), serviceStatusProvider);
-        }
-    }
-
-    @Override
-    public Set<ApplicationInstanceReference> getAllApplicationInstanceReferences() {
-        return modelGenerator.toApplicationInstanceReferenceSet(duperModelManager.getApplicationInfos());
-    }
-
-    @Override
-    public Optional<ApplicationInstance> getApplication(HostName hostname) {
-        Optional<ApplicationInfo> applicationInfo = getApplicationInfo(hostname);
-        if (applicationInfo.isEmpty()) {
-            return Optional.empty();
-        }
-
-        return Optional.of(modelGenerator.toApplicationInstance(applicationInfo.get(), serviceStatusProvider));
-    }
-
-    @Override
-    public Optional<ApplicationInstance> getApplication(ApplicationInstanceReference reference) {
-        return getApplicationInfo(reference)
-                .map(applicationInfo -> modelGenerator.toApplicationInstance(applicationInfo, serviceStatusProvider));
-    }
-
-    @Override
-    public Optional<ApplicationInstance> getApplicationNarrowedTo(HostName hostname) {
-        Optional<ApplicationInfo> applicationInfo = getApplicationInfo(hostname);
-        if (applicationInfo.isEmpty()) {
-            return Optional.empty();
-        }
-
-        return Optional.of(modelGenerator.toApplicationNarrowedToHost(
-                applicationInfo.get(), hostname, serviceStatusProvider));
-    }
-
-    @Override
-    public Map<HostName, List<ServiceInstance>> getServicesByHostname() {
-        return getServiceModelSnapshot().getServiceInstancesByHostName();
-    }
-
-    @Override
-    public void registerListener(ServiceHostListener listener) {
-        var duperModelListener = ServiceHostListenerAdapter.asDuperModelListener(listener, modelGenerator);
-        duperModelManager.registerListener(duperModelListener);
-    }
-
-    private Optional<ApplicationInfo> getApplicationInfo(ApplicationInstanceReference reference) {
-        ApplicationId applicationId = ApplicationInstanceGenerator.toApplicationId(reference);
-        return duperModelManager.getApplicationInfo(applicationId);
-    }
-
-    private Optional<ApplicationInfo> getApplicationInfo(HostName hostname) {
-        // The duper model uses HostName from config.provision, which is more natural than applicationmodel.
-        var configProvisionHostname = com.yahoo.config.provision.HostName.from(hostname.s());
-        return duperModelManager.getApplicationInfo(configProvisionHostname);
-    }
-}
diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceMonitorImpl.java b/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceMonitorImpl.java
index 7fd50e2f941..7c0f291e121 100644
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceMonitorImpl.java
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceMonitorImpl.java
@@ -2,6 +2,8 @@
 package com.yahoo.vespa.service.model;
 
 import com.google.inject.Inject;
+import com.yahoo.config.model.api.ApplicationInfo;
+import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.Timer;
@@ -9,13 +11,13 @@ import com.yahoo.vespa.applicationmodel.ApplicationInstance;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.applicationmodel.ServiceInstance;
-import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.service.duper.DuperModelManager;
+import com.yahoo.vespa.service.manager.MonitorManager;
 import com.yahoo.vespa.service.manager.UnionMonitorManager;
 import com.yahoo.vespa.service.monitor.ServiceHostListener;
 import com.yahoo.vespa.service.monitor.ServiceModel;
 import com.yahoo.vespa.service.monitor.ServiceMonitor;
+import com.yahoo.vespa.service.monitor.ServiceStatusProvider;
 
 import java.util.List;
 import java.util.Map;
@@ -24,62 +26,93 @@ import java.util.Set;
 
 public class ServiceMonitorImpl implements ServiceMonitor {
 
-    private final ServiceMonitor delegate;
+    private final ServiceMonitorMetrics metrics;
+    private final DuperModelManager duperModelManager;
+    private final ModelGenerator modelGenerator;
+    private final ServiceStatusProvider serviceStatusProvider;
 
     @Inject
     public ServiceMonitorImpl(DuperModelManager duperModelManager,
                               UnionMonitorManager monitorManager,
                               Metric metric,
                               Timer timer,
-                              Zone zone,
-                              FlagSource flagSource) {
-        duperModelManager.registerListener(monitorManager);
-
-        ServiceMonitor serviceMonitor = new ServiceModelProvider(
-                monitorManager,
+                              Zone zone) {
+        this(monitorManager,
                 new ServiceMonitorMetrics(metric, timer),
                 duperModelManager,
-                new ModelGenerator(zone));
-
-        if (Flags.SERVICE_MODEL_CACHE.bindTo(flagSource).value()) {
-            delegate = new ServiceModelCache(serviceMonitor, timer);
-        } else {
-            delegate = serviceMonitor;
-        }
+                new ModelGenerator(zone)
+        );
     }
 
+    ServiceMonitorImpl(MonitorManager monitorManager,
+                       ServiceMonitorMetrics metrics,
+                       DuperModelManager duperModelManager,
+                       ModelGenerator modelGenerator) {
+        this.serviceStatusProvider = monitorManager;
+        this.metrics = metrics;
+        this.duperModelManager = duperModelManager;
+        this.modelGenerator = modelGenerator;
+
+        duperModelManager.registerListener(monitorManager);
+    }
     @Override
     public ServiceModel getServiceModelSnapshot() {
-        return delegate.getServiceModelSnapshot();
+        try (LatencyMeasurement measurement = metrics.startServiceModelSnapshotLatencyMeasurement()) {
+            return modelGenerator.toServiceModel(duperModelManager.getApplicationInfos(), serviceStatusProvider);
+        }
     }
 
     @Override
     public Set<ApplicationInstanceReference> getAllApplicationInstanceReferences() {
-        return delegate.getAllApplicationInstanceReferences();
+        return modelGenerator.toApplicationInstanceReferenceSet(duperModelManager.getApplicationInfos());
     }
 
     @Override
     public Optional<ApplicationInstance> getApplication(HostName hostname) {
-        return delegate.getApplication(hostname);
+        Optional<ApplicationInfo> applicationInfo = getApplicationInfo(hostname);
+        if (applicationInfo.isEmpty()) {
+            return Optional.empty();
+        }
+
+        return Optional.of(modelGenerator.toApplicationInstance(applicationInfo.get(), serviceStatusProvider));
     }
 
     @Override
     public Optional<ApplicationInstance> getApplication(ApplicationInstanceReference reference) {
-        return delegate.getApplication(reference);
+        return getApplicationInfo(reference)
+                .map(applicationInfo -> modelGenerator.toApplicationInstance(applicationInfo, serviceStatusProvider));
     }
 
     @Override
     public Optional<ApplicationInstance> getApplicationNarrowedTo(HostName hostname) {
-        return delegate.getApplicationNarrowedTo(hostname);
+        Optional<ApplicationInfo> applicationInfo = getApplicationInfo(hostname);
+        if (applicationInfo.isEmpty()) {
+            return Optional.empty();
+        }
+
+        return Optional.of(modelGenerator.toApplicationNarrowedToHost(
+                applicationInfo.get(), hostname, serviceStatusProvider));
     }
 
     @Override
     public Map<HostName, List<ServiceInstance>> getServicesByHostname() {
-        return delegate.getServicesByHostname();
+        return getServiceModelSnapshot().getServiceInstancesByHostName();
     }
 
     @Override
     public void registerListener(ServiceHostListener listener) {
-        delegate.registerListener(listener);
+        var duperModelListener = ServiceHostListenerAdapter.asDuperModelListener(listener, modelGenerator);
+        duperModelManager.registerListener(duperModelListener);
+    }
+
+    private Optional<ApplicationInfo> getApplicationInfo(ApplicationInstanceReference reference) {
+        ApplicationId applicationId = ApplicationInstanceGenerator.toApplicationId(reference);
+        return duperModelManager.getApplicationInfo(applicationId);
+    }
+
+    private Optional<ApplicationInfo> getApplicationInfo(HostName hostname) {
+        // The duper model uses HostName from config.provision, which is more natural than applicationmodel.
+        var configProvisionHostname = com.yahoo.config.provision.HostName.from(hostname.s());
+        return duperModelManager.getApplicationInfo(configProvisionHostname);
     }
 }
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/model/ServiceModelCacheTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/model/ServiceModelCacheTest.java
deleted file mode 100644
index 8649fec8e7e..00000000000
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/model/ServiceModelCacheTest.java
+++ /dev/null
@@ -1,58 +0,0 @@
-// 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.model;
-
-import com.yahoo.jdisc.Timer;
-import com.yahoo.vespa.service.monitor.ServiceModel;
-import com.yahoo.vespa.service.monitor.ServiceMonitor;
-import org.junit.Test;
-
-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 {
-    private final ServiceMonitor expensiveServiceMonitor = mock(ServiceMonitor.class);
-    private final Timer timer = mock(Timer.class);
-    private final ServiceModelCache cache = new ServiceModelCache(expensiveServiceMonitor, timer);
-
-    @Test
-    public void sanityCheck() {
-        ServiceModel serviceModel = mock(ServiceModel.class);
-        when(expensiveServiceMonitor.getServiceModelSnapshot()).thenReturn(serviceModel);
-
-        long timeMillis = 0;
-        when(timer.currentTimeMillis()).thenReturn(timeMillis);
-
-        // Will always populate cache the first time
-        ServiceModel actualServiceModel = cache.getServiceModelSnapshot();
-        assertTrue(actualServiceModel == serviceModel);
-        verify(expensiveServiceMonitor, times(1)).getServiceModelSnapshot();
-
-        // Cache hit
-        timeMillis += ServiceModelCache.EXPIRY_MILLIS / 2;
-        when(timer.currentTimeMillis()).thenReturn(timeMillis);
-        actualServiceModel = cache.getServiceModelSnapshot();
-        assertTrue(actualServiceModel == serviceModel);
-
-        // Cache expired
-        timeMillis += ServiceModelCache.EXPIRY_MILLIS + 1;
-        when(timer.currentTimeMillis()).thenReturn(timeMillis);
-
-        ServiceModel serviceModel2 = mock(ServiceModel.class);
-        when(expensiveServiceMonitor.getServiceModelSnapshot()).thenReturn(serviceModel2);
-
-        actualServiceModel = cache.getServiceModelSnapshot();
-        assertTrue(actualServiceModel == serviceModel2);
-        // '2' because it's cumulative with '1' from the first times(1).
-        verify(expensiveServiceMonitor, times(2)).getServiceModelSnapshot();
-
-        // Cache hit #2
-        timeMillis += 1;
-        when(timer.currentTimeMillis()).thenReturn(timeMillis);
-        actualServiceModel = cache.getServiceModelSnapshot();
-        assertTrue(actualServiceModel == serviceModel2);
-    }
-}
\ No newline at end of file
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/model/ServiceModelProviderTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/model/ServiceMonitorImplTest.java
index 3b98e50bf9a..5b0855d0be5 100644
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/model/ServiceModelProviderTest.java
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/model/ServiceMonitorImplTest.java
@@ -16,13 +16,13 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-public class ServiceModelProviderTest {
+public class ServiceMonitorImplTest {
     @Test
     public void sanityCheck() {
         SlobrokMonitorManagerImpl slobrokMonitorManager = mock(SlobrokMonitorManagerImpl.class);
         DuperModelManager duperModelManager = mock(DuperModelManager.class);
         ModelGenerator modelGenerator = mock(ModelGenerator.class);
-        ServiceModelProvider provider = new ServiceModelProvider(
+        ServiceMonitorImpl serviceMonitor = new ServiceMonitorImpl(
                 slobrokMonitorManager,
                 mock(ServiceMonitorMetrics.class),
                 duperModelManager,
@@ -34,7 +34,7 @@ public class ServiceModelProviderTest {
                 .collect(Collectors.toList());
         when(duperModelManager.getApplicationInfos()).thenReturn(applications);
 
-        ServiceModel serviceModel = provider.getServiceModelSnapshot();
+        ServiceModel serviceModel = serviceMonitor.getServiceModelSnapshot();
         verify(duperModelManager, times(1)).getApplicationInfos();
         verify(modelGenerator).toServiceModel(applications, slobrokMonitorManager);
     }