Introduce simple container watchdog

This watchdog uses information from the reference count framework in
jdisc_core. Contains no use of System.gc() or WeakReference.

6 files changed, 232 insertions, 1 deletions

diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java
index b7cab923454..6c09cdf92f7 100644
--- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java
@@ -8,6 +8,7 @@ import com.yahoo.jdisc.application.ContainerBuilder;
 import com.yahoo.jdisc.application.ContainerThread;
 import com.yahoo.jdisc.application.OsgiFramework;
 import com.yahoo.jdisc.service.CurrentContainer;
+import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics;
 
 import java.util.concurrent.ThreadFactory;
 
@@ -28,6 +29,7 @@ class ApplicationEnvironmentModule extends AbstractModule {
         bind(CurrentContainer.class).toInstance(loader);
         bind(OsgiFramework.class).toInstance(loader.osgiFramework());
         bind(ThreadFactory.class).to(ContainerThread.Factory.class);
+        bind(ContainerWatchdogMetrics.class).toInstance(loader.getContainerWatchdogMetrics());
     }
 
     @Provides
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java
index 81eb5815a01..3ba48ffd8cd 100644
--- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java
@@ -15,6 +15,7 @@ import com.yahoo.jdisc.application.OsgiFramework;
 import com.yahoo.jdisc.application.OsgiHeader;
 import com.yahoo.jdisc.service.ContainerNotReadyException;
 import com.yahoo.jdisc.service.CurrentContainer;
+import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleException;
@@ -40,6 +41,7 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C
     private final AtomicReference<ActiveContainer> containerRef = new AtomicReference<>();
     private final Object appLock = new Object();
     private final List<Bundle> appBundles = new ArrayList<>();
+    private final ContainerWatchdog watchdog = new ContainerWatchdog();
     private Application application;
     private ApplicationInUseTracker applicationInUseTracker;
 
@@ -68,6 +70,7 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C
                 next.retainReference(applicationInUseTracker);
             }
 
+            watchdog.onContainerActivation(next);
             prev = containerRef.getAndSet(next);
             if (prev == null) {
                 return null;
@@ -193,8 +196,9 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C
     public void destroy() {
         log.finer("Destroying application loader.");
         try {
+            watchdog.close();
             osgiFramework.stop();
-        } catch (BundleException e) {
+        } catch (BundleException | InterruptedException e) {
             e.printStackTrace();
         }
     }
@@ -205,6 +209,10 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C
         }
     }
 
+    public ContainerWatchdogMetrics getContainerWatchdogMetrics() {
+        return watchdog;
+    }
+
     public OsgiFramework osgiFramework() {
         return osgiFramework;
     }
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerWatchdog.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerWatchdog.java
new file mode 100644
index 00000000000..30aa0028465
--- /dev/null
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerWatchdog.java
@@ -0,0 +1,126 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.jdisc.core;
+
+import com.yahoo.jdisc.Metric;
+import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics;
+
+import java.time.Clock;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Logger;
+
+/**
+ * A watchdog that monitors all deactivated {@link ActiveContainer} instances with the purpose of detecting stale containers.
+ *
+ * @author bjorncs
+ */
+class ContainerWatchdog implements ContainerWatchdogMetrics, AutoCloseable {
+
+    static final Duration GRACE_PERIOD = Duration.ofMinutes(30);
+    static final Duration UPDATE_PERIOD = Duration.ofMinutes(5);
+
+    private static final Logger log = Logger.getLogger(ContainerWatchdog.class.getName());
+
+    private final Object monitor = new Object();
+    private final List<DeactivatedContainer> deactivatedContainers = new LinkedList<>();
+    private final ScheduledExecutorService scheduler;
+    private final Clock clock;
+
+    private ActiveContainer currentContainer;
+    private Instant currentContainerActivationTime;
+    private int numStaleContainers;
+
+    ContainerWatchdog() {
+        this(new ScheduledThreadPoolExecutor(
+                     1,
+                     runnable -> {
+                         Thread thread = new Thread(runnable, "container-watchdog");
+                         thread.setDaemon(true);
+                         return thread;
+                     }),
+             Clock.systemUTC());
+    }
+
+    ContainerWatchdog(ScheduledExecutorService scheduler, Clock clock) {
+        this.scheduler = scheduler;
+        this.clock = clock;
+        scheduler.scheduleAtFixedRate(
+                this::monitorDeactivatedContainers, UPDATE_PERIOD.getSeconds(), UPDATE_PERIOD.getSeconds(), TimeUnit.SECONDS);
+    }
+
+    @Override
+    public void emitMetrics(Metric metric) {
+        int numStaleContainers;
+        synchronized (monitor) {
+            numStaleContainers = this.numStaleContainers;
+        }
+        metric.set(TOTAL_DEACTIVATED_CONTAINERS, numStaleContainers, null);
+    }
+
+    @Override
+    public void close() throws InterruptedException {
+        scheduler.shutdownNow();
+        scheduler.awaitTermination(1, TimeUnit.MINUTES);
+        synchronized (monitor) {
+            deactivatedContainers.clear();
+            currentContainer = null;
+            currentContainerActivationTime = null;
+        }
+    }
+
+    void onContainerActivation(ActiveContainer nextContainer) {
+        synchronized (monitor) {
+            if (currentContainer != null) {
+                deactivatedContainers.add(
+                        new DeactivatedContainer(currentContainer, currentContainerActivationTime, clock.instant()));
+            }
+            currentContainer = nextContainer;
+            currentContainerActivationTime = clock.instant();
+        }
+    }
+
+    void monitorDeactivatedContainers() {
+        synchronized (monitor) {
+            int numStaleContainer = 0;
+            Iterator<DeactivatedContainer> iterator = deactivatedContainers.iterator();
+            while (iterator.hasNext()) {
+                DeactivatedContainer container = iterator.next();
+                int refCount = container.instance.retainCount();
+                if (refCount == 0) {
+                    iterator.remove();
+                    break;
+                }
+                if (isPastGracePeriod(container)) {
+                    ++numStaleContainer;
+                    log.warning(
+                            String.format(
+                                    "Deactivated container still alive: instance=%s, activated=%s, deactivated=%s, ref-count=%d",
+                                    container.instance.toString(), container.timeActivated, container.timeDeactivated, refCount));
+                }
+            }
+            this.numStaleContainers = numStaleContainer;
+        }
+    }
+
+    private boolean isPastGracePeriod(DeactivatedContainer container) {
+        return clock.instant().isAfter(container.timeDeactivated.plus(GRACE_PERIOD));
+    }
+
+    private static class DeactivatedContainer {
+        final ActiveContainer instance;
+        final Instant timeActivated;
+        final Instant timeDeactivated;
+
+        DeactivatedContainer(ActiveContainer instance, Instant timeActivated, Instant timeDeactivated) {
+            this.instance = instance;
+            this.timeActivated = timeActivated;
+            this.timeDeactivated = timeDeactivated;
+        }
+    }
+}
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ContainerWatchdogMetrics.java b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ContainerWatchdogMetrics.java
new file mode 100644
index 00000000000..2a286aef990
--- /dev/null
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ContainerWatchdogMetrics.java
@@ -0,0 +1,18 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.jdisc.statistics;
+
+import com.yahoo.jdisc.Metric;
+import com.yahoo.jdisc.core.ActiveContainer;
+
+/**
+ * Tracks statistics on stale {@link ActiveContainer} instances.
+ *
+ * @author bjorncs
+ */
+public interface ContainerWatchdogMetrics {
+
+    String TOTAL_DEACTIVATED_CONTAINERS = "jdisc.deactivated_containers.total";
+
+    void emitMetrics(Metric metric);
+
+}
diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java
new file mode 100644
index 00000000000..cebe03c5103
--- /dev/null
+++ b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java
@@ -0,0 +1,4 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+@com.yahoo.osgi.annotation.ExportPackage
+package com.yahoo.jdisc.statistics;
diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java
new file mode 100644
index 00000000000..94e7c3a1d22
--- /dev/null
+++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java
@@ -0,0 +1,73 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.jdisc.core;
+
+import com.yahoo.jdisc.Metric;
+import com.yahoo.jdisc.test.TestDriver;
+import com.yahoo.test.ManualClock;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+
+/**
+ * @author bjorncs
+ */
+public class ContainerWatchdogTest {
+
+    @Test
+    public void watchdog_counts_stale_container() {
+        TestDriver driver = TestDriver.newSimpleApplicationInstanceWithoutOsgi();
+        ManualClock clock = new ManualClock(Instant.EPOCH);
+        DummyMetric metric = new DummyMetric();
+        ContainerWatchdog watchdog = new ContainerWatchdog(mock(ScheduledExecutorService.class), clock);
+
+        ActiveContainer containerWithoutRetainedResources = new ActiveContainer(driver.newContainerBuilder());
+
+        watchdog.onContainerActivation(containerWithoutRetainedResources);
+        assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric));
+
+        clock.advance(Duration.ofHours(1));
+        watchdog.onContainerActivation(null);
+        assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric));
+
+        clock.advance(ContainerWatchdog.GRACE_PERIOD);
+        assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric));
+
+        clock.advance(Duration.ofSeconds(1));
+        assertEquals(1, runMonitorStepAndGetStaleContainerCount(watchdog, metric));
+
+        containerWithoutRetainedResources.release();
+        assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric));
+    }
+
+    private static int runMonitorStepAndGetStaleContainerCount(ContainerWatchdog watchdog, DummyMetric metric) {
+        watchdog.monitorDeactivatedContainers();
+        watchdog.emitMetrics(metric);
+        return metric.value;
+    }
+
+    private static class DummyMetric implements Metric {
+        int value;
+
+        @Override
+        public void set(String key, Number val, Context ctx) {
+            this.value = val.intValue();
+        }
+
+        @Override
+        public void add(String key, Number val, Context ctx) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public Context createContext(Map<String, ?> properties) {
+            throw new UnsupportedOperationException();
+        }
+    }
+}
+