Merge pull request #1973 from metamx/cachemonitor-optional-injection

CacheMonitor - make cache injection optional

server/src/main/java/io/druid/client/cache/CacheMonitor.java

@@ -22,15 +22,17 @@ import com.metamx.emitter.service.ServiceEmitter;
 import com.metamx.emitter.service.ServiceMetricEvent;
 import com.metamx.metrics.AbstractMonitor;
 
-/**
- */
 public class CacheMonitor extends AbstractMonitor
 {
-  private final Cache cache;
+  // package private for tests
+  volatile Cache cache;
 
   private volatile CacheStats prevCacheStats = null;
 
-  @Inject
+  public CacheMonitor()
+  {
+  }
+
   public CacheMonitor(
       Cache cache
   )
@@ -38,20 +40,30 @@ public class CacheMonitor extends AbstractMonitor
     this.cache = cache;
   }
 
+  // make it possible to enable CacheMonitor even if cache is not bound
+  // (e.g. some index tasks may have a cache, others may not)
+  @Inject(optional = true)
+  public void setCache(Cache cache)
+  {
+    this.cache = cache;
+  }
+
   @Override
   public boolean doMonitor(ServiceEmitter emitter)
   {
-    final CacheStats currCacheStats = cache.getStats();
-    final CacheStats deltaCacheStats = currCacheStats.delta(prevCacheStats);
+    if (cache != null) {
+      final CacheStats currCacheStats = cache.getStats();
+      final CacheStats deltaCacheStats = currCacheStats.delta(prevCacheStats);
 
-    final ServiceMetricEvent.Builder builder = new ServiceMetricEvent.Builder();
-    emitStats(emitter, "query/cache/delta", deltaCacheStats, builder);
-    emitStats(emitter, "query/cache/total", currCacheStats, builder);
+      final ServiceMetricEvent.Builder builder = new ServiceMetricEvent.Builder();
+      emitStats(emitter, "query/cache/delta", deltaCacheStats, builder);
+      emitStats(emitter, "query/cache/total", currCacheStats, builder);
 
-    prevCacheStats = currCacheStats;
+      prevCacheStats = currCacheStats;
 
-    // Any custom cache statistics that need monitoring
-    cache.doMonitor(emitter);
+      // Any custom cache statistics that need monitoring
+      cache.doMonitor(emitter);
+    }
     return true;
   }
 
@@ -62,14 +74,16 @@ public class CacheMonitor extends AbstractMonitor
       ServiceMetricEvent.Builder builder
   )
   {
-    emitter.emit(builder.build(String.format("%s/numEntries", metricPrefix), cacheStats.getNumEntries()));
-    emitter.emit(builder.build(String.format("%s/sizeBytes", metricPrefix), cacheStats.getSizeInBytes()));
-    emitter.emit(builder.build(String.format("%s/hits", metricPrefix), cacheStats.getNumHits()));
-    emitter.emit(builder.build(String.format("%s/misses", metricPrefix), cacheStats.getNumMisses()));
-    emitter.emit(builder.build(String.format("%s/evictions", metricPrefix), cacheStats.getNumEvictions()));
-    emitter.emit(builder.build(String.format("%s/hitRate", metricPrefix), cacheStats.hitRate()));
-    emitter.emit(builder.build(String.format("%s/averageBytes", metricPrefix), cacheStats.averageBytes()));
-    emitter.emit(builder.build(String.format("%s/timeouts", metricPrefix), cacheStats.getNumTimeouts()));
-    emitter.emit(builder.build(String.format("%s/errors", metricPrefix), cacheStats.getNumErrors()));
+    if (cache != null) {
+      emitter.emit(builder.build(String.format("%s/numEntries", metricPrefix), cacheStats.getNumEntries()));
+      emitter.emit(builder.build(String.format("%s/sizeBytes", metricPrefix), cacheStats.getSizeInBytes()));
+      emitter.emit(builder.build(String.format("%s/hits", metricPrefix), cacheStats.getNumHits()));
+      emitter.emit(builder.build(String.format("%s/misses", metricPrefix), cacheStats.getNumMisses()));
+      emitter.emit(builder.build(String.format("%s/evictions", metricPrefix), cacheStats.getNumEvictions()));
+      emitter.emit(builder.build(String.format("%s/hitRate", metricPrefix), cacheStats.hitRate()));
+      emitter.emit(builder.build(String.format("%s/averageBytes", metricPrefix), cacheStats.averageBytes()));
+      emitter.emit(builder.build(String.format("%s/timeouts", metricPrefix), cacheStats.getNumTimeouts()));
+      emitter.emit(builder.build(String.format("%s/errors", metricPrefix), cacheStats.getNumErrors()));
+    }
   }
 }

server/src/test/java/io/druid/client/cache/CacheMonitorTest.java

@@ -0,0 +1,75 @@
+/*
+ * Licensed to Metamarkets Group Inc. (Metamarkets) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  Metamarkets licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package io.druid.client.cache;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.Binder;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Module;
+import io.druid.guice.GuiceInjectors;
+import io.druid.guice.JsonConfigProvider;
+import io.druid.guice.annotations.Self;
+import io.druid.initialization.Initialization;
+import io.druid.server.DruidNode;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class CacheMonitorTest
+{
+  @Test
+  public void testOptionalInject() throws Exception
+  {
+    Injector injector = Initialization.makeInjectorWithModules(GuiceInjectors.makeStartupInjector(), ImmutableList.of(
+        new Module() {
+          @Override
+          public void configure(Binder binder)
+          {
+            JsonConfigProvider.bindInstance(
+                binder, Key.get(DruidNode.class, Self.class), new DruidNode("test-inject", null, null)
+            );
+          }
+        }
+    ));
+
+    CacheMonitor monitor = injector.getInstance(CacheMonitor.class);
+    Assert.assertNull(monitor.cache);
+  }
+
+  @Test
+  public void testInject() throws Exception
+  {
+    Injector injector = Initialization.makeInjectorWithModules(GuiceInjectors.makeStartupInjector(), ImmutableList.of(
+        new Module() {
+          @Override
+          public void configure(Binder binder)
+          {
+            JsonConfigProvider.bindInstance(
+                binder, Key.get(DruidNode.class, Self.class), new DruidNode("test-inject", null, null)
+            );
+            binder.bind(Cache.class).toInstance(MapCache.create(0));
+          }
+        }
+    ));
+
+    CacheMonitor monitor = injector.getInstance(CacheMonitor.class);
+    Assert.assertNotNull(monitor.cache);
+  }
+}