diff --git a/pom.xml b/pom.xml index c0990c9..55f4c56 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ com.urbanairship datacube - 1.5.0-SNAPSHOT + 1.5.1-SNAPSHOT jar datacube diff --git a/src/main/java/com/urbanairship/datacube/idservices/CachingIdService.java b/src/main/java/com/urbanairship/datacube/idservices/CachingIdService.java index 7530b5e..5ee3221 100644 --- a/src/main/java/com/urbanairship/datacube/idservices/CachingIdService.java +++ b/src/main/java/com/urbanairship/datacube/idservices/CachingIdService.java @@ -11,8 +11,12 @@ import com.urbanairship.datacube.IdService; import com.yammer.metrics.Metrics; import com.yammer.metrics.core.Gauge; +import com.yammer.metrics.core.Meter; +import com.yammer.metrics.core.Timer; +import com.yammer.metrics.core.TimerContext; import java.io.IOException; +import java.util.concurrent.TimeUnit; /** * An IdService that wraps around another IdService and caches its results. Calls to getOrCreateId() are @@ -25,7 +29,16 @@ public class CachingIdService implements IdService { private final Cache cache; private final IdService wrappedIdService; + private final Timer idGetTime; + private final boolean cacheMisses; + private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; + private final Meter cachedNullResult; + public CachingIdService(int numCached, final IdService wrappedIdService, final String cacheName) { + this(numCached, wrappedIdService, cacheName, false); + } + + public CachingIdService(int numCached, final IdService wrappedIdService, final String cacheName, boolean cacheMisses) { this.wrappedIdService = wrappedIdService; this.cache = CacheBuilder.newBuilder() .maximumSize(numCached) @@ -46,6 +59,11 @@ public Double value() { return cache.stats().hitRate(); } }); + + this.idGetTime = Metrics.newTimer(CachingIdService.class, + "id_get", cacheName, TimeUnit.MILLISECONDS, TimeUnit.SECONDS); + cachedNullResult = Metrics.newMeter(CachingIdService.class, cacheName, "cache null", TimeUnit.SECONDS); + this.cacheMisses = cacheMisses; } @@ -64,17 +82,27 @@ public byte[] getOrCreateId(int dimensionNum, byte[] bytes, int numIdBytes) thro @Override public Optional getId(int dimensionNum, byte[] bytes, int numIdBytes) throws IOException, InterruptedException { - final Key key = new Key(dimensionNum, new BoxedByteArray(bytes), numIdBytes); - final byte[] cachedVal = cache.getIfPresent(key); - - if (cachedVal == null) { - final Optional id = wrappedIdService.getId(dimensionNum, bytes, numIdBytes); - if (id.isPresent()) { - cache.put(key, id.get()); + final TimerContext time = idGetTime.time(); + try { + final Key key = new Key(dimensionNum, new BoxedByteArray(bytes), numIdBytes); + final byte[] cachedVal = cache.getIfPresent(key); + + if (cachedVal == null) { + final Optional id = wrappedIdService.getId(dimensionNum, bytes, numIdBytes); + if (id.isPresent()) { + cache.put(key, id.get()); + } else if (cacheMisses) { + cachedNullResult.mark(); + cache.put(key, EMPTY_BYTE_ARRAY); + } + return id; + } else if (cachedVal == EMPTY_BYTE_ARRAY) { + return Optional.absent(); + } else { + return Optional.of(cachedVal); } - return id; - } else { - return Optional.of(cachedVal); + } finally { + time.stop(); } } diff --git a/src/test/java/com/urbanairship/datacube/IdServiceTests.java b/src/test/java/com/urbanairship/datacube/IdServiceTests.java index 74d17e1..f1c1a5a 100644 --- a/src/test/java/com/urbanairship/datacube/IdServiceTests.java +++ b/src/test/java/com/urbanairship/datacube/IdServiceTests.java @@ -4,17 +4,22 @@ package com.urbanairship.datacube; +import com.google.common.base.Optional; import com.google.common.math.LongMath; +import com.urbanairship.datacube.idservices.CachingIdService; import org.junit.Assert; +import org.junit.Test; +import java.io.IOException; import java.util.HashSet; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; public class IdServiceTests { public static void basicTest(IdService idService) throws Exception { final int numFieldBytes = 5; - + // Different inputs should always produce different outputs (non-repeating ids) Set idsSeen = new HashSet(); for(int i=0; i<500; i++) { @@ -23,46 +28,102 @@ public static void basicTest(IdService idService) throws Exception { BoxedByteArray newBox = new BoxedByteArray(newId); Assert.assertTrue("ID was repeated: " + newBox, idsSeen.add(newBox)); } - + // The same input should produce the same output byte[] id1 = idService.getOrCreateId(1, Util.longToBytes(10), numFieldBytes); byte[] id2 = idService.getOrCreateId(1, Util.longToBytes(10), numFieldBytes); Assert.assertEquals(numFieldBytes, id1.length); Assert.assertArrayEquals(id1, id2); } - + /** * Generating 2^(fieldbits) unique IDs should work, then generating one more should raise an * exception because no more IDs were available. - * + * */ - public static void testExhaustion(IdService idService, int numFieldBytes, int dimensionNum) + public static void testExhaustion(IdService idService, int numFieldBytes, int dimensionNum) throws Exception { int numFieldBits = numFieldBytes * 8; - + long numToGenerate = LongMath.pow(2, numFieldBits); long i=0; for(; i TimeUnit.SECONDS.toNanos(5)) { + } catch (RuntimeException e) { + if (System.nanoTime() - startTimeNanos > TimeUnit.SECONDS.toNanos(5)) { Assert.fail("Took too long to fail"); } } } + + @Test + public void testCacheMissingOff() throws IOException, InterruptedException { + final CountingIdService wrappedIdService = new CountingIdService(); + final CachingIdService lol = new CachingIdService(2, wrappedIdService, "lol", false); + lol.getId(-1, CountingIdService.unknown, 0); + lol.getId(-1, CountingIdService.unknown, 0); + lol.getId(-1, CountingIdService.known, 0); + lol.getId(-1, CountingIdService.known, 0); + // should not have been cached, so accessed backing store twice + Assert.assertEquals(2, wrappedIdService.unKnownCount.get()); + // should have been cached, so accessed backing store once + Assert.assertEquals(1, wrappedIdService.knownCount.get()); + } + + @Test + public void testCacheMissingOn() throws IOException, InterruptedException { + final CountingIdService wrappedIdService = new CountingIdService(); + final CachingIdService lol = new CachingIdService(2, wrappedIdService, "lol", true); + lol.getId(-1, CountingIdService.unknown, 0); + lol.getId(-1, CountingIdService.unknown, 0); + lol.getId(-1, CountingIdService.known, 0); + lol.getId(-1, CountingIdService.known, 0); + // should have been cached, so accessed backing store once + Assert.assertEquals(1, wrappedIdService.unKnownCount.get()); + // should have been cached, so accessed backing store once + Assert.assertEquals(1, wrappedIdService.knownCount.get()); + } + + private static final class CountingIdService implements IdService { + + public static final byte[] known = "known".getBytes(); + public static final byte[] unknown = "unknown".getBytes(); + + public final AtomicInteger knownCount = new AtomicInteger(0); + public final AtomicInteger unKnownCount = new AtomicInteger(0); + + @Override + public byte[] getOrCreateId(int dimensionNum, byte[] input, int numIdBytes) throws IOException, InterruptedException { + throw new RuntimeException("the feature this class tests doesn't make sense with getOrCreateId"); + } + + @Override + public Optional getId(int dimensionNum, byte[] input, int numIdBytes) throws IOException, InterruptedException { + if (input == known) { + knownCount.incrementAndGet(); + return Optional.of(known); + } else if (input == unknown) { + unKnownCount.incrementAndGet(); + return Optional.absent(); + } else { + throw new RuntimeException("only use the two values for testing plz"); + } + } + + } }