From 1032d81fceaa9d2dd8d65904eee720d116e893f0 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 19 Sep 2017 10:45:59 -0700 Subject: [PATCH 1/9] Added statistics support --- src/main/java/org/dataloader/DataLoader.java | 20 +++ .../org/dataloader/DataLoaderOptions.java | 32 ++++- .../stats/SimpleStatisticsCollector.java | 40 ++++++ .../java/org/dataloader/stats/Statistics.java | 36 +++++ .../dataloader/stats/StatisticsCollector.java | 33 +++++ .../org/dataloader/stats/StatisticsImpl.java | 57 ++++++++ .../stats/ThreadLocalStatisticsCollector.java | 72 ++++++++++ .../java/org/dataloader/DataLoaderTest.java | 109 ++++++++++++++ .../stats/StatisticsCollectorTest.java | 134 ++++++++++++++++++ 9 files changed, 531 insertions(+), 2 deletions(-) create mode 100644 src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java create mode 100644 src/main/java/org/dataloader/stats/Statistics.java create mode 100644 src/main/java/org/dataloader/stats/StatisticsCollector.java create mode 100644 src/main/java/org/dataloader/stats/StatisticsImpl.java create mode 100644 src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java create mode 100644 src/test/java/org/dataloader/stats/StatisticsCollectorTest.java diff --git a/src/main/java/org/dataloader/DataLoader.java b/src/main/java/org/dataloader/DataLoader.java index 056b8b4..5ceb3be 100644 --- a/src/main/java/org/dataloader/DataLoader.java +++ b/src/main/java/org/dataloader/DataLoader.java @@ -17,6 +17,8 @@ package org.dataloader; import org.dataloader.impl.CompletableFutureKit; +import org.dataloader.stats.Statistics; +import org.dataloader.stats.StatisticsCollector; import java.util.ArrayList; import java.util.Collection; @@ -64,6 +66,7 @@ public class DataLoader { private final DataLoaderOptions loaderOptions; private final CacheMap> futureCache; private final Map> loaderQueue; + private final StatisticsCollector stats; /** * Creates new DataLoader with the specified batch loader function and default options @@ -153,6 +156,7 @@ public DataLoader(BatchLoader batchLoadFunction, DataLoaderOptions options this.futureCache = determineCacheMap(loaderOptions); // order of keys matter in data loader this.loaderQueue = new LinkedHashMap<>(); + this.stats = nonNull(this.loaderOptions.getStatisticsCollector()); } @SuppressWarnings("unchecked") @@ -173,8 +177,11 @@ private CacheMap> determineCacheMap(DataLoaderOptio */ public CompletableFuture load(K key) { Object cacheKey = getCacheKey(nonNull(key)); + stats.incrementLoadCount(); + synchronized (futureCache) { if (loaderOptions.cachingEnabled() && futureCache.containsKey(cacheKey)) { + stats.incrementCacheHitCount(); return futureCache.get(cacheKey); } } @@ -191,6 +198,7 @@ public CompletableFuture load(K key) { .toCompletableFuture(); future = batchedLoad .thenApply(list -> list.get(0)); + stats.incrementBatchLoadCount(); } if (loaderOptions.cachingEnabled()) { synchronized (futureCache) { @@ -291,6 +299,7 @@ private CompletableFuture> sliceIntoBatchesOfBatches(List keys, List< @SuppressWarnings("unchecked") private CompletableFuture> dispatchQueueBatch(List keys, List> queuedFutures) { + stats.incrementBatchLoadCount(); return batchLoadFunction.load(keys) .toCompletableFuture() .thenApply(values -> { @@ -441,4 +450,15 @@ public Object getCacheKey(K key) { return loaderOptions.cacheKeyFunction().isPresent() ? loaderOptions.cacheKeyFunction().get().getKey(key) : key; } + + /** + * Gets the statistics associated with this data loader. These will have been gather via + * the {@link org.dataloader.stats.StatisticsCollector} passed in via {@link DataLoaderOptions#getStatisticsCollector()} + * + * @return statistics for this data loader + */ + public Statistics getStatistics() { + return stats.getStatistics(); + } + } diff --git a/src/main/java/org/dataloader/DataLoaderOptions.java b/src/main/java/org/dataloader/DataLoaderOptions.java index 02d10ff..8d55f84 100644 --- a/src/main/java/org/dataloader/DataLoaderOptions.java +++ b/src/main/java/org/dataloader/DataLoaderOptions.java @@ -16,9 +16,13 @@ package org.dataloader; -import org.dataloader.impl.Assertions; +import org.dataloader.stats.SimpleStatisticsCollector; +import org.dataloader.stats.StatisticsCollector; import java.util.Optional; +import java.util.function.Supplier; + +import static org.dataloader.impl.Assertions.nonNull; /** * Configuration options for {@link DataLoader} instances. @@ -32,6 +36,7 @@ public class DataLoaderOptions { private CacheKey cacheKeyFunction; private CacheMap cacheMap; private int maxBatchSize; + private Supplier statisticsCollector; /** * Creates a new data loader options with default settings. @@ -40,6 +45,7 @@ public DataLoaderOptions() { batchingEnabled = true; cachingEnabled = true; maxBatchSize = -1; + statisticsCollector = SimpleStatisticsCollector::new; } /** @@ -48,12 +54,13 @@ public DataLoaderOptions() { * @param other the other options instance */ public DataLoaderOptions(DataLoaderOptions other) { - Assertions.nonNull(other); + nonNull(other); this.batchingEnabled = other.batchingEnabled; this.cachingEnabled = other.cachingEnabled; this.cacheKeyFunction = other.cacheKeyFunction; this.cacheMap = other.cacheMap; this.maxBatchSize = other.maxBatchSize; + this.statisticsCollector = other.statisticsCollector; } /** @@ -173,4 +180,25 @@ public DataLoaderOptions setMaxBatchSize(int maxBatchSize) { this.maxBatchSize = maxBatchSize; return this; } + + /** + * @return the statistics collector to use with these options + */ + public StatisticsCollector getStatisticsCollector() { + return nonNull(this.statisticsCollector.get()); + } + + /** + * Sets the statistics collector supplier that will be used with these data loader options. Since it uses + * the supplier pattern, you can create a new statistics collector on each call or you can reuse + * a common value + * + * @return the data loader options for fluent coding + */ + public DataLoaderOptions setStatisticsCollector(Supplier statisticsCollector) { + this.statisticsCollector = nonNull(statisticsCollector); + return this; + } + + } diff --git a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java new file mode 100644 index 0000000..4acaaa1 --- /dev/null +++ b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java @@ -0,0 +1,40 @@ +package org.dataloader.stats; + +import java.util.concurrent.atomic.AtomicLong; + +/** + * This simple collector uses {@link java.util.concurrent.atomic.AtomicLong}s to collect + * statistics + * + * @see org.dataloader.stats.StatisticsCollector + */ +public class SimpleStatisticsCollector implements StatisticsCollector { + private final AtomicLong loadCount = new AtomicLong(); + private final AtomicLong batchLoadCount = new AtomicLong(); + private final AtomicLong cacheHitCount = new AtomicLong(); + + @Override + public long incrementLoadCount() { + return loadCount.incrementAndGet(); + } + + @Override + public long incrementBatchLoadCount() { + return batchLoadCount.incrementAndGet(); + } + + @Override + public long incrementCacheHitCount() { + return cacheHitCount.incrementAndGet(); + } + + @Override + public Statistics getStatistics() { + return new StatisticsImpl(loadCount.get(), batchLoadCount.get(), cacheHitCount.get()); + } + + @Override + public String toString() { + return getStatistics().toString(); + } +} diff --git a/src/main/java/org/dataloader/stats/Statistics.java b/src/main/java/org/dataloader/stats/Statistics.java new file mode 100644 index 0000000..73b6f88 --- /dev/null +++ b/src/main/java/org/dataloader/stats/Statistics.java @@ -0,0 +1,36 @@ +package org.dataloader.stats; + +/** + * A simple set of statistics of {@link org.dataloader.DataLoader} operations + */ +public interface Statistics { + /** + * @return the number of times {@link org.dataloader.DataLoader#load(Object)} has been called + */ + long getLoadCount(); + + /** + * @return the number of times the {@link org.dataloader.DataLoader} batch loader function has been called + */ + long getBatchLoadCount(); + + /** + * @return batchLoadCount / loadCount + */ + float getBatchLoadRatio(); + + /** + * @return the number of times {@link org.dataloader.DataLoader#load(Object)} resulted in a cache hit + */ + long getCacheHitCount(); + + /** + * @return then number of times we missed the cache during {@link org.dataloader.DataLoader#load(Object)} + */ + long getCacheMissCount(); + + /** + * @return cacheHits / loadCount + */ + float getCacheHitRatio(); +} diff --git a/src/main/java/org/dataloader/stats/StatisticsCollector.java b/src/main/java/org/dataloader/stats/StatisticsCollector.java new file mode 100644 index 0000000..e74839c --- /dev/null +++ b/src/main/java/org/dataloader/stats/StatisticsCollector.java @@ -0,0 +1,33 @@ +package org.dataloader.stats; + +/** + * This allows statistics to be collected for {@link org.dataloader.DataLoader} operations + */ +public interface StatisticsCollector { + + /** + * Called to increment the number of loads + * + * @return the current value after increment + */ + long incrementLoadCount(); + + /** + * Called to increment the number of batch loads + * + * @return the current value after increment + */ + long incrementBatchLoadCount(); + + /** + * Called to increment the number of cache hits + * + * @return the current value after increment + */ + long incrementCacheHitCount(); + + /** + * @return the statistics that have been gathered up to this point in time + */ + Statistics getStatistics(); +} diff --git a/src/main/java/org/dataloader/stats/StatisticsImpl.java b/src/main/java/org/dataloader/stats/StatisticsImpl.java new file mode 100644 index 0000000..c24a1cf --- /dev/null +++ b/src/main/java/org/dataloader/stats/StatisticsImpl.java @@ -0,0 +1,57 @@ +package org.dataloader.stats; + +public class StatisticsImpl implements Statistics { + + private final long loadCount; + private final long batchLoadCount; + private final long cacheHitCount; + + public StatisticsImpl(long loadCount, long batchLoadCount, long cacheHitCount) { + this.loadCount = loadCount; + this.batchLoadCount = batchLoadCount; + this.cacheHitCount = cacheHitCount; + } + + @Override + public long getLoadCount() { + return loadCount; + } + + @Override + public long getBatchLoadCount() { + return batchLoadCount; + } + + @Override + public float getBatchLoadRatio() { + return ratio(batchLoadCount, loadCount); + } + + private float ratio(long numerator, long denominator) { + return denominator == 0 ? 0f : ((float) numerator) / ((float) denominator); + } + + @Override + public long getCacheHitCount() { + return cacheHitCount; + } + + @Override + public long getCacheMissCount() { + return loadCount - cacheHitCount; + } + + @Override + public float getCacheHitRatio() { + return ratio(cacheHitCount, loadCount); + } + + @Override + public String toString() { + return "StatisticsImpl{" + + "loadCount=" + loadCount + + ", batchLoadCount=" + batchLoadCount + + ", cacheHitCount=" + cacheHitCount + + '}'; + } +} diff --git a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java new file mode 100644 index 0000000..5cb3e8a --- /dev/null +++ b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java @@ -0,0 +1,72 @@ +package org.dataloader.stats; + +/** + * This can collect statistics per thread as well as in an overall sense. This allows you to snapshot stats for a web request say + * as well as all requests. + * + * You will want to call {@link #resetThread()} to clean up the thread local aspects of this object per request thread + * + * @see org.dataloader.stats.StatisticsCollector + */ +public class ThreadLocalStatisticsCollector implements StatisticsCollector { + + private static final ThreadLocal collector = ThreadLocal.withInitial(SimpleStatisticsCollector::new); + + private final SimpleStatisticsCollector overallCollector = new SimpleStatisticsCollector(); + + /** + * Removes the underlying thread local value for this current thread. This is a way to reset the thread local + * values for the current thread and start afresh + * + * @return this collector for fluent coding + */ + public ThreadLocalStatisticsCollector resetThread() { + collector.remove(); + return this; + } + + @Override + public long incrementLoadCount() { + overallCollector.incrementLoadCount(); + return collector.get().incrementLoadCount(); + } + + @Override + public long incrementBatchLoadCount() { + overallCollector.incrementBatchLoadCount(); + return collector.get().incrementBatchLoadCount(); + } + + @Override + public long incrementCacheHitCount() { + overallCollector.incrementCacheHitCount(); + return collector.get().incrementCacheHitCount(); + } + + /** + * This returns the statistics for this thread. + * + * @return this thread's statistics + */ + @Override + public Statistics getStatistics() { + return collector.get().getStatistics(); + } + + /** + * This returns the overall statistics, that is not per thread but for the life of this object + * + * @return overall statistics + */ + public Statistics getOverallStatistics() { + return overallCollector.getStatistics(); + } + + @Override + public String toString() { + return "ThreadLocalStatisticsCollector{" + + "thread=" + getStatistics().toString() + + "overallCollector=" + overallCollector.getStatistics().toString() + + '}'; + } +} diff --git a/src/test/java/org/dataloader/DataLoaderTest.java b/src/test/java/org/dataloader/DataLoaderTest.java index 4f9f44c..9f7890c 100644 --- a/src/test/java/org/dataloader/DataLoaderTest.java +++ b/src/test/java/org/dataloader/DataLoaderTest.java @@ -19,6 +19,9 @@ import org.dataloader.fixtures.User; import org.dataloader.fixtures.UserManager; import org.dataloader.impl.CompletableFutureKit; +import org.dataloader.stats.SimpleStatisticsCollector; +import org.dataloader.stats.Statistics; +import org.dataloader.stats.StatisticsCollector; import org.hamcrest.Matchers; import org.junit.Test; @@ -998,6 +1001,112 @@ public void should_handle_Trys_coming_back_from_batchLoader() throws Exception { assertThat(batchKeyCalls, equalTo(singletonList(asList("A", "B", "bang")))); } + @Test + public void stats_are_collected_by_default() throws Exception { + BatchLoader batchLoader = CompletableFuture::completedFuture; + DataLoader loader = new DataLoader<>(batchLoader); + + loader.load("A"); + loader.load("B"); + loader.loadMany(asList("C", "D")); + + Statistics stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(0L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.load("A"); + loader.load("B"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(6L)); + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(2L)); + } + + + @Test + public void stats_are_collected_with_specified_collector() throws Exception { + // lets prime it with some numbers so we know its ours + StatisticsCollector collector = new SimpleStatisticsCollector(); + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + + BatchLoader batchLoader = CompletableFuture::completedFuture; + DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector(() -> collector); + DataLoader loader = new DataLoader<>(batchLoader, loaderOptions); + + loader.load("A"); + loader.load("B"); + loader.loadMany(asList("C", "D")); + + Statistics stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(5L)); // previously primed with 1 + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(5L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.load("A"); + loader.load("B"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(7L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getCacheHitCount(), equalTo(2L)); + } + + @Test + public void stats_are_collected_with_caching_disabled() throws Exception { + StatisticsCollector collector = new SimpleStatisticsCollector(); + + BatchLoader batchLoader = CompletableFuture::completedFuture; + DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector(() -> collector).setCachingEnabled(false); + DataLoader loader = new DataLoader<>(batchLoader, loaderOptions); + + loader.load("A"); + loader.load("B"); + loader.loadMany(asList("C", "D")); + + Statistics stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(0L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.load("A"); + loader.load("B"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(6L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + } + private static CacheKey getJsonObjectCacheMapFn() { return key -> key.stream() .map(entry -> entry.getKey() + ":" + entry.getValue()) diff --git a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java new file mode 100644 index 0000000..dbece15 --- /dev/null +++ b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java @@ -0,0 +1,134 @@ +package org.dataloader.stats; + +import org.junit.Test; + +import java.util.concurrent.CompletableFuture; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +public class StatisticsCollectorTest { + + @Test + public void basic_collection() throws Exception { + StatisticsCollector collector = new SimpleStatisticsCollector(); + + assertThat(collector.getStatistics().getLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); + + + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + collector.incrementCacheHitCount(); + + assertThat(collector.getStatistics().getLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(1L)); + assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); + } + + @Test + public void ratios_work() throws Exception { + + StatisticsCollector collector = new SimpleStatisticsCollector(); + + collector.incrementLoadCount(); + + Statistics stats = collector.getStatistics(); + assertThat(stats.getBatchLoadRatio(), equalTo(0f)); + assertThat(stats.getCacheHitRatio(), equalTo(0f)); + + + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + + stats = collector.getStatistics(); + assertThat(stats.getBatchLoadRatio(), equalTo(1f / 4f)); + + + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementCacheHitCount(); + collector.incrementCacheHitCount(); + + stats = collector.getStatistics(); + assertThat(stats.getCacheHitRatio(), equalTo(2f / 7f)); + } + + @Test + public void thread_local_collection() throws Exception { + + final ThreadLocalStatisticsCollector collector = new ThreadLocalStatisticsCollector(); + + assertThat(collector.getStatistics().getLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(0L)); + + + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + collector.incrementCacheHitCount(); + + assertThat(collector.getStatistics().getLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(1L)); + + assertThat(collector.getOverallStatistics().getLoadCount(), equalTo(1L)); + assertThat(collector.getOverallStatistics().getBatchLoadCount(), equalTo(1L)); + assertThat(collector.getOverallStatistics().getCacheHitCount(), equalTo(1L)); + + CompletableFuture.supplyAsync(() -> { + + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + collector.incrementCacheHitCount(); + + // per thread stats here + assertThat(collector.getStatistics().getLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(1L)); + + // overall stats + assertThat(collector.getOverallStatistics().getLoadCount(), equalTo(2L)); + assertThat(collector.getOverallStatistics().getBatchLoadCount(), equalTo(2L)); + assertThat(collector.getOverallStatistics().getCacheHitCount(), equalTo(2L)); + + return null; + }).join(); + + // back on this main thread + + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + collector.incrementCacheHitCount(); + + // per thread stats here + assertThat(collector.getStatistics().getLoadCount(), equalTo(2L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(2L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(2L)); + + // overall stats + assertThat(collector.getOverallStatistics().getLoadCount(), equalTo(3L)); + assertThat(collector.getOverallStatistics().getBatchLoadCount(), equalTo(3L)); + assertThat(collector.getOverallStatistics().getCacheHitCount(), equalTo(3L)); + + + // stats can be reset per thread + collector.resetThread(); + + // thread is reset + assertThat(collector.getStatistics().getLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(0L)); + + // but not overall stats + assertThat(collector.getOverallStatistics().getLoadCount(), equalTo(3L)); + assertThat(collector.getOverallStatistics().getBatchLoadCount(), equalTo(3L)); + assertThat(collector.getOverallStatistics().getCacheHitCount(), equalTo(3L)); + } +} \ No newline at end of file From b86bba08327927ff05b16e292c1c6f97c576bbc5 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 19 Sep 2017 12:49:37 -0700 Subject: [PATCH 2/9] added delegating stats collector --- .../stats/DelegatingStatisticsCollector.java | 55 +++++++++++++++++++ .../stats/ThreadLocalStatisticsCollector.java | 5 +- .../stats/StatisticsCollectorTest.java | 32 +++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java diff --git a/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java new file mode 100644 index 0000000..6e71610 --- /dev/null +++ b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java @@ -0,0 +1,55 @@ +package org.dataloader.stats; + +import static org.dataloader.impl.Assertions.nonNull; + +/** + * This statistics collector keeps dataloader statistics AND also calls the delegate + * collector at the same time. This allows you to keep a specific set of statistics + * and also delegate the calls onto another collector. + */ +public class DelegatingStatisticsCollector implements StatisticsCollector { + + private final StatisticsCollector collector = new SimpleStatisticsCollector(); + private final StatisticsCollector delegateCollector; + + /** + * @param delegateCollector a non null delegate collector + */ + public DelegatingStatisticsCollector(StatisticsCollector delegateCollector) { + this.delegateCollector = nonNull(delegateCollector); + } + + @Override + public long incrementLoadCount() { + delegateCollector.incrementLoadCount(); + return collector.incrementLoadCount(); + } + + @Override + public long incrementBatchLoadCount() { + delegateCollector.incrementBatchLoadCount(); + return collector.incrementBatchLoadCount(); + } + + @Override + public long incrementCacheHitCount() { + delegateCollector.incrementCacheHitCount(); + return collector.incrementCacheHitCount(); + } + + /** + * @return the statistics of the this collector (and not its delegate) + */ + @Override + public Statistics getStatistics() { + return collector.getStatistics(); + } + + /** + * @return the statistics of the delegate + */ + public Statistics getDelegateStatistics() { + return delegateCollector.getStatistics(); + } + +} diff --git a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java index 5cb3e8a..50ff1e6 100644 --- a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java @@ -4,7 +4,10 @@ * This can collect statistics per thread as well as in an overall sense. This allows you to snapshot stats for a web request say * as well as all requests. * - * You will want to call {@link #resetThread()} to clean up the thread local aspects of this object per request thread + * You will want to call {@link #resetThread()} to clean up the thread local aspects of this object per request thread. + * + * ThreadLocals have their place in the Java world but be careful on how you use them. If you don't clean them up on "request boundaries" + * then you WILL have misleading statistics. * * @see org.dataloader.stats.StatisticsCollector */ diff --git a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java index dbece15..78b078e 100644 --- a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java +++ b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java @@ -131,4 +131,36 @@ public void thread_local_collection() throws Exception { assertThat(collector.getOverallStatistics().getBatchLoadCount(), equalTo(3L)); assertThat(collector.getOverallStatistics().getCacheHitCount(), equalTo(3L)); } + + @Test + public void delegating_collector_works() throws Exception { + SimpleStatisticsCollector delegate = new SimpleStatisticsCollector(); + DelegatingStatisticsCollector collector = new DelegatingStatisticsCollector(delegate); + + assertThat(collector.getStatistics().getLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); + + + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + collector.incrementCacheHitCount(); + + assertThat(collector.getStatistics().getLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(1L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(1L)); + assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); + + assertThat(collector.getDelegateStatistics().getLoadCount(), equalTo(1L)); + assertThat(collector.getDelegateStatistics().getBatchLoadCount(), equalTo(1L)); + assertThat(collector.getDelegateStatistics().getCacheHitCount(), equalTo(1L)); + assertThat(collector.getDelegateStatistics().getCacheMissCount(), equalTo(0L)); + + assertThat(delegate.getStatistics().getLoadCount(), equalTo(1L)); + assertThat(delegate.getStatistics().getBatchLoadCount(), equalTo(1L)); + assertThat(delegate.getStatistics().getCacheHitCount(), equalTo(1L)); + assertThat(delegate.getStatistics().getCacheMissCount(), equalTo(0L)); + + } } \ No newline at end of file From f69448f7ce2c49bec99675f45911d4cca4578b90 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 19 Sep 2017 17:19:52 -0700 Subject: [PATCH 3/9] Added error counts to stats --- src/main/java/org/dataloader/DataLoader.java | 27 ++- .../java/org/dataloader/impl/Assertions.java | 4 + .../stats/DelegatingStatisticsCollector.java | 12 ++ .../stats/SimpleStatisticsCollector.java | 14 +- .../java/org/dataloader/stats/Statistics.java | 24 ++- .../dataloader/stats/StatisticsCollector.java | 14 ++ .../org/dataloader/stats/StatisticsImpl.java | 28 ++- .../stats/ThreadLocalStatisticsCollector.java | 12 ++ .../org/dataloader/DataLoaderStatsTest.java | 187 ++++++++++++++++++ .../java/org/dataloader/DataLoaderTest.java | 108 ---------- .../stats/StatisticsCollectorTest.java | 33 ++++ 11 files changed, 342 insertions(+), 121 deletions(-) create mode 100644 src/test/java/org/dataloader/DataLoaderStatsTest.java diff --git a/src/main/java/org/dataloader/DataLoader.java b/src/main/java/org/dataloader/DataLoader.java index 5ceb3be..cf07a8c 100644 --- a/src/main/java/org/dataloader/DataLoader.java +++ b/src/main/java/org/dataloader/DataLoader.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import java.util.stream.Collectors; import static java.util.Collections.emptyList; @@ -300,7 +301,13 @@ private CompletableFuture> sliceIntoBatchesOfBatches(List keys, List< @SuppressWarnings("unchecked") private CompletableFuture> dispatchQueueBatch(List keys, List> queuedFutures) { stats.incrementBatchLoadCount(); - return batchLoadFunction.load(keys) + CompletionStage> batchLoad; + try { + batchLoad = nonNull(batchLoadFunction.load(keys), "Your batch loader function MUST return a non null CompletionStage promise"); + } catch (Exception e) { + batchLoad = CompletableFutureKit.failedFuture(e); + } + return batchLoad .toCompletableFuture() .thenApply(values -> { assertState(keys.size() == values.size(), "The size of the promised values MUST be the same size as the key list"); @@ -309,13 +316,20 @@ private CompletableFuture> dispatchQueueBatch(List keys, List future = queuedFutures.get(idx); if (value instanceof Throwable) { + stats.incrementLoadErrorCount(); future.completeExceptionally((Throwable) value); // we don't clear the cached view of this entry to avoid // frequently loading the same error } else if (value instanceof Try) { // we allow the batch loader to return a Try so we can better represent a computation // that might have worked or not. - handleTry((Try) value, future); + Try tryValue = (Try) value; + if (tryValue.isSuccess()) { + future.complete(tryValue.get()); + } else { + stats.incrementLoadErrorCount(); + future.completeExceptionally(tryValue.getThrowable()); + } } else { V val = (V) value; future.complete(val); @@ -323,6 +337,7 @@ private CompletableFuture> dispatchQueueBatch(List keys, List { + stats.incrementBatchLoadExceptionCount(); for (int idx = 0; idx < queuedFutures.size(); idx++) { K key = keys.get(idx); CompletableFuture future = queuedFutures.get(idx); @@ -334,14 +349,6 @@ private CompletableFuture> dispatchQueueBatch(List keys, List vTry, CompletableFuture future) { - if (vTry.isSuccess()) { - future.complete(vTry.get()); - } else { - future.completeExceptionally(vTry.getThrowable()); - } - } - /** * Normally {@link #dispatch()} is an asynchronous operation but this version will 'join' on the * results if dispatch and wait for them to complete. If the {@link CompletableFuture} callbacks make more diff --git a/src/main/java/org/dataloader/impl/Assertions.java b/src/main/java/org/dataloader/impl/Assertions.java index 86dac33..ad1a1ef 100644 --- a/src/main/java/org/dataloader/impl/Assertions.java +++ b/src/main/java/org/dataloader/impl/Assertions.java @@ -14,6 +14,10 @@ public static T nonNull(T t) { return Objects.requireNonNull(t, "nonNull object required"); } + public static T nonNull(T t, String message) { + return Objects.requireNonNull(t, message); + } + private static class AssertionException extends IllegalStateException { public AssertionException(String message) { super(message); diff --git a/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java index 6e71610..5b74d5c 100644 --- a/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java @@ -37,6 +37,18 @@ public long incrementCacheHitCount() { return collector.incrementCacheHitCount(); } + @Override + public long incrementLoadErrorCount() { + delegateCollector.incrementLoadErrorCount(); + return collector.incrementLoadErrorCount(); + } + + @Override + public long incrementBatchLoadExceptionCount() { + delegateCollector.incrementBatchLoadExceptionCount(); + return collector.incrementBatchLoadExceptionCount(); + } + /** * @return the statistics of the this collector (and not its delegate) */ diff --git a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java index 4acaaa1..3d78739 100644 --- a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java @@ -12,6 +12,8 @@ public class SimpleStatisticsCollector implements StatisticsCollector { private final AtomicLong loadCount = new AtomicLong(); private final AtomicLong batchLoadCount = new AtomicLong(); private final AtomicLong cacheHitCount = new AtomicLong(); + private final AtomicLong batchLoadExceptionCount = new AtomicLong(); + private final AtomicLong loadErrorCount = new AtomicLong(); @Override public long incrementLoadCount() { @@ -28,9 +30,19 @@ public long incrementCacheHitCount() { return cacheHitCount.incrementAndGet(); } + @Override + public long incrementLoadErrorCount() { + return loadErrorCount.incrementAndGet(); + } + + @Override + public long incrementBatchLoadExceptionCount() { + return batchLoadExceptionCount.incrementAndGet(); + } + @Override public Statistics getStatistics() { - return new StatisticsImpl(loadCount.get(), batchLoadCount.get(), cacheHitCount.get()); + return new StatisticsImpl(loadCount.get(), batchLoadCount.get(), cacheHitCount.get(), batchLoadExceptionCount.get(), loadErrorCount.get()); } @Override diff --git a/src/main/java/org/dataloader/stats/Statistics.java b/src/main/java/org/dataloader/stats/Statistics.java index 73b6f88..5f9e56d 100644 --- a/src/main/java/org/dataloader/stats/Statistics.java +++ b/src/main/java/org/dataloader/stats/Statistics.java @@ -1,7 +1,7 @@ package org.dataloader.stats; /** - * A simple set of statistics of {@link org.dataloader.DataLoader} operations + * A simple set of statistics about {@link org.dataloader.DataLoader} operations */ public interface Statistics { /** @@ -33,4 +33,26 @@ public interface Statistics { * @return cacheHits / loadCount */ float getCacheHitRatio(); + + /** + * @return the number of times the {@link org.dataloader.DataLoader} batch loader function throw an exception when trying to get any values + */ + long getBatchLoadExceptionCount(); + + /** + * @return batchLoadExceptionCount / loadCount + */ + float getBatchLoadExceptionRatio(); + + /** + * @return the number of times the {@link org.dataloader.DataLoader} batch loader function return an specific object that was in error + */ + long getLoadErrorCount(); + + + /** + * @return loadErrorCount / loadCount + */ + float getLoadErrorRatio(); + } diff --git a/src/main/java/org/dataloader/stats/StatisticsCollector.java b/src/main/java/org/dataloader/stats/StatisticsCollector.java index e74839c..fcec15e 100644 --- a/src/main/java/org/dataloader/stats/StatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/StatisticsCollector.java @@ -12,6 +12,13 @@ public interface StatisticsCollector { */ long incrementLoadCount(); + /** + * Called to increment the number of loads that resulted in an object deemed in error + * + * @return the current value after increment + */ + long incrementLoadErrorCount(); + /** * Called to increment the number of batch loads * @@ -19,6 +26,13 @@ public interface StatisticsCollector { */ long incrementBatchLoadCount(); + /** + * Called to increment the number of batch loads exceptions + * + * @return the current value after increment + */ + long incrementBatchLoadExceptionCount(); + /** * Called to increment the number of cache hits * diff --git a/src/main/java/org/dataloader/stats/StatisticsImpl.java b/src/main/java/org/dataloader/stats/StatisticsImpl.java index c24a1cf..4c2ac18 100644 --- a/src/main/java/org/dataloader/stats/StatisticsImpl.java +++ b/src/main/java/org/dataloader/stats/StatisticsImpl.java @@ -5,11 +5,15 @@ public class StatisticsImpl implements Statistics { private final long loadCount; private final long batchLoadCount; private final long cacheHitCount; + private final long batchLoadExceptionCount; + private final long loadErrorCount; - public StatisticsImpl(long loadCount, long batchLoadCount, long cacheHitCount) { + public StatisticsImpl(long loadCount, long batchLoadCount, long cacheHitCount, long batchLoadExceptionCount, long loadErrorCount) { this.loadCount = loadCount; this.batchLoadCount = batchLoadCount; this.cacheHitCount = cacheHitCount; + this.batchLoadExceptionCount = batchLoadExceptionCount; + this.loadErrorCount = loadErrorCount; } @Override @@ -46,12 +50,34 @@ public float getCacheHitRatio() { return ratio(cacheHitCount, loadCount); } + @Override + public long getBatchLoadExceptionCount() { + return batchLoadExceptionCount; + } + + @Override + public long getLoadErrorCount() { + return loadErrorCount; + } + + @Override + public float getBatchLoadExceptionRatio() { + return ratio(batchLoadExceptionCount, loadCount); + } + + @Override + public float getLoadErrorRatio() { + return ratio(loadErrorCount, loadCount); + } + @Override public String toString() { return "StatisticsImpl{" + "loadCount=" + loadCount + ", batchLoadCount=" + batchLoadCount + ", cacheHitCount=" + cacheHitCount + + ", batchLoadExceptionCount=" + batchLoadExceptionCount + + ", loadErrorCount=" + loadErrorCount + '}'; } } diff --git a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java index 50ff1e6..faacf18 100644 --- a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java @@ -46,6 +46,18 @@ public long incrementCacheHitCount() { return collector.get().incrementCacheHitCount(); } + @Override + public long incrementLoadErrorCount() { + overallCollector.incrementLoadErrorCount(); + return collector.get().incrementLoadErrorCount(); + } + + @Override + public long incrementBatchLoadExceptionCount() { + overallCollector.incrementBatchLoadExceptionCount(); + return collector.get().incrementBatchLoadExceptionCount(); + } + /** * This returns the statistics for this thread. * diff --git a/src/test/java/org/dataloader/DataLoaderStatsTest.java b/src/test/java/org/dataloader/DataLoaderStatsTest.java new file mode 100644 index 0000000..a7f3101 --- /dev/null +++ b/src/test/java/org/dataloader/DataLoaderStatsTest.java @@ -0,0 +1,187 @@ +package org.dataloader; + +import org.dataloader.impl.CompletableFutureKit; +import org.dataloader.stats.SimpleStatisticsCollector; +import org.dataloader.stats.Statistics; +import org.dataloader.stats.StatisticsCollector; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; + +import static java.util.Arrays.asList; +import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Tests related to stats. DataLoaderTest is getting to big and needs refactoring + */ +public class DataLoaderStatsTest { + + @Test + public void stats_are_collected_by_default() throws Exception { + BatchLoader batchLoader = CompletableFuture::completedFuture; + DataLoader loader = new DataLoader<>(batchLoader); + + loader.load("A"); + loader.load("B"); + loader.loadMany(asList("C", "D")); + + Statistics stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(0L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.load("A"); + loader.load("B"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(6L)); + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(2L)); + } + + + @Test + public void stats_are_collected_with_specified_collector() throws Exception { + // lets prime it with some numbers so we know its ours + StatisticsCollector collector = new SimpleStatisticsCollector(); + collector.incrementLoadCount(); + collector.incrementBatchLoadCount(); + + BatchLoader batchLoader = CompletableFuture::completedFuture; + DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector(() -> collector); + DataLoader loader = new DataLoader<>(batchLoader, loaderOptions); + + loader.load("A"); + loader.load("B"); + loader.loadMany(asList("C", "D")); + + Statistics stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(5L)); // previously primed with 1 + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(5L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.load("A"); + loader.load("B"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(7L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getCacheHitCount(), equalTo(2L)); + } + + @Test + public void stats_are_collected_with_caching_disabled() throws Exception { + StatisticsCollector collector = new SimpleStatisticsCollector(); + + BatchLoader batchLoader = CompletableFuture::completedFuture; + DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector(() -> collector).setCachingEnabled(false); + DataLoader loader = new DataLoader<>(batchLoader, loaderOptions); + + loader.load("A"); + loader.load("B"); + loader.loadMany(asList("C", "D")); + + Statistics stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(0L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + + loader.load("A"); + loader.load("B"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(6L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getCacheHitCount(), equalTo(0L)); + } + + BatchLoader> batchLoaderThatBlows = keys -> { + List> values = new ArrayList<>(); + for (String key : keys) { + if (key.startsWith("exception")) { + return CompletableFutureKit.failedFuture(new RuntimeException(key)); + } else if (key.startsWith("bang")) { + throw new RuntimeException(key); + } else if (key.startsWith("error")) { + values.add(Try.failed(new RuntimeException(key))); + } else { + values.add(Try.succeeded(key)); + } + } + return completedFuture(values); + }; + + @Test + public void stats_are_collected_on_exceptions() throws Exception { + DataLoader loader = DataLoader.newDataLoaderWithTry(batchLoaderThatBlows); + + loader.load("A"); + loader.load("exception"); + + Statistics stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(2L)); + assertThat(stats.getBatchLoadExceptionCount(), equalTo(0L)); + assertThat(stats.getLoadErrorCount(), equalTo(0L)); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(2L)); + assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getBatchLoadExceptionCount(), equalTo(1L)); + assertThat(stats.getLoadErrorCount(), equalTo(0L)); + + loader.load("error1"); + loader.load("error2"); + loader.load("error3"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(5L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getBatchLoadExceptionCount(), equalTo(1L)); + assertThat(stats.getLoadErrorCount(), equalTo(3L)); + + loader.load("bang"); + + loader.dispatch(); + + stats = loader.getStatistics(); + assertThat(stats.getLoadCount(), equalTo(6L)); + assertThat(stats.getBatchLoadCount(), equalTo(3L)); + assertThat(stats.getBatchLoadExceptionCount(), equalTo(2L)); + assertThat(stats.getLoadErrorCount(), equalTo(3L)); + } +} diff --git a/src/test/java/org/dataloader/DataLoaderTest.java b/src/test/java/org/dataloader/DataLoaderTest.java index 9f7890c..da9ebff 100644 --- a/src/test/java/org/dataloader/DataLoaderTest.java +++ b/src/test/java/org/dataloader/DataLoaderTest.java @@ -19,9 +19,6 @@ import org.dataloader.fixtures.User; import org.dataloader.fixtures.UserManager; import org.dataloader.impl.CompletableFutureKit; -import org.dataloader.stats.SimpleStatisticsCollector; -import org.dataloader.stats.Statistics; -import org.dataloader.stats.StatisticsCollector; import org.hamcrest.Matchers; import org.junit.Test; @@ -1001,111 +998,6 @@ public void should_handle_Trys_coming_back_from_batchLoader() throws Exception { assertThat(batchKeyCalls, equalTo(singletonList(asList("A", "B", "bang")))); } - @Test - public void stats_are_collected_by_default() throws Exception { - BatchLoader batchLoader = CompletableFuture::completedFuture; - DataLoader loader = new DataLoader<>(batchLoader); - - loader.load("A"); - loader.load("B"); - loader.loadMany(asList("C", "D")); - - Statistics stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(4L)); - assertThat(stats.getBatchLoadCount(), equalTo(0L)); - assertThat(stats.getCacheHitCount(), equalTo(0L)); - - loader.dispatch(); - - stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(4L)); - assertThat(stats.getBatchLoadCount(), equalTo(1L)); - assertThat(stats.getCacheHitCount(), equalTo(0L)); - - loader.load("A"); - loader.load("B"); - - loader.dispatch(); - - stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(6L)); - assertThat(stats.getBatchLoadCount(), equalTo(1L)); - assertThat(stats.getCacheHitCount(), equalTo(2L)); - } - - - @Test - public void stats_are_collected_with_specified_collector() throws Exception { - // lets prime it with some numbers so we know its ours - StatisticsCollector collector = new SimpleStatisticsCollector(); - collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); - - BatchLoader batchLoader = CompletableFuture::completedFuture; - DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector(() -> collector); - DataLoader loader = new DataLoader<>(batchLoader, loaderOptions); - - loader.load("A"); - loader.load("B"); - loader.loadMany(asList("C", "D")); - - Statistics stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(5L)); // previously primed with 1 - assertThat(stats.getBatchLoadCount(), equalTo(1L)); - assertThat(stats.getCacheHitCount(), equalTo(0L)); - - loader.dispatch(); - - stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(5L)); - assertThat(stats.getBatchLoadCount(), equalTo(2L)); - assertThat(stats.getCacheHitCount(), equalTo(0L)); - - loader.load("A"); - loader.load("B"); - - loader.dispatch(); - - stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(7L)); - assertThat(stats.getBatchLoadCount(), equalTo(2L)); - assertThat(stats.getCacheHitCount(), equalTo(2L)); - } - - @Test - public void stats_are_collected_with_caching_disabled() throws Exception { - StatisticsCollector collector = new SimpleStatisticsCollector(); - - BatchLoader batchLoader = CompletableFuture::completedFuture; - DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector(() -> collector).setCachingEnabled(false); - DataLoader loader = new DataLoader<>(batchLoader, loaderOptions); - - loader.load("A"); - loader.load("B"); - loader.loadMany(asList("C", "D")); - - Statistics stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(4L)); - assertThat(stats.getBatchLoadCount(), equalTo(0L)); - assertThat(stats.getCacheHitCount(), equalTo(0L)); - - loader.dispatch(); - - stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(4L)); - assertThat(stats.getBatchLoadCount(), equalTo(1L)); - assertThat(stats.getCacheHitCount(), equalTo(0L)); - - loader.load("A"); - loader.load("B"); - - loader.dispatch(); - - stats = loader.getStatistics(); - assertThat(stats.getLoadCount(), equalTo(6L)); - assertThat(stats.getBatchLoadCount(), equalTo(2L)); - assertThat(stats.getCacheHitCount(), equalTo(0L)); - } private static CacheKey getJsonObjectCacheMapFn() { return key -> key.stream() diff --git a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java index 78b078e..ba98ddc 100644 --- a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java +++ b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java @@ -17,16 +17,22 @@ public void basic_collection() throws Exception { assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(0L)); assertThat(collector.getStatistics().getCacheHitCount(), equalTo(0L)); assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadExceptionCount(), equalTo(0L)); + assertThat(collector.getStatistics().getLoadErrorCount(), equalTo(0L)); collector.incrementLoadCount(); collector.incrementBatchLoadCount(); collector.incrementCacheHitCount(); + collector.incrementBatchLoadExceptionCount(); + collector.incrementLoadErrorCount(); assertThat(collector.getStatistics().getLoadCount(), equalTo(1L)); assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(1L)); assertThat(collector.getStatistics().getCacheHitCount(), equalTo(1L)); assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadExceptionCount(), equalTo(1L)); + assertThat(collector.getStatistics().getLoadErrorCount(), equalTo(1L)); } @Test @@ -58,6 +64,25 @@ public void ratios_work() throws Exception { stats = collector.getStatistics(); assertThat(stats.getCacheHitRatio(), equalTo(2f / 7f)); + + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementBatchLoadExceptionCount(); + collector.incrementBatchLoadExceptionCount(); + + stats = collector.getStatistics(); + assertThat(stats.getBatchLoadExceptionRatio(), equalTo(2f / 10f)); + + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementLoadCount(); + collector.incrementLoadErrorCount(); + collector.incrementLoadErrorCount(); + collector.incrementLoadErrorCount(); + + stats = collector.getStatistics(); + assertThat(stats.getLoadErrorRatio(), equalTo(3f / 13f)); } @Test @@ -146,21 +171,29 @@ public void delegating_collector_works() throws Exception { collector.incrementLoadCount(); collector.incrementBatchLoadCount(); collector.incrementCacheHitCount(); + collector.incrementBatchLoadExceptionCount(); + collector.incrementLoadErrorCount(); assertThat(collector.getStatistics().getLoadCount(), equalTo(1L)); assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(1L)); assertThat(collector.getStatistics().getCacheHitCount(), equalTo(1L)); assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadExceptionCount(), equalTo(1L)); + assertThat(collector.getStatistics().getLoadErrorCount(), equalTo(1L)); assertThat(collector.getDelegateStatistics().getLoadCount(), equalTo(1L)); assertThat(collector.getDelegateStatistics().getBatchLoadCount(), equalTo(1L)); assertThat(collector.getDelegateStatistics().getCacheHitCount(), equalTo(1L)); assertThat(collector.getDelegateStatistics().getCacheMissCount(), equalTo(0L)); + assertThat(collector.getDelegateStatistics().getBatchLoadExceptionCount(), equalTo(1L)); + assertThat(collector.getDelegateStatistics().getLoadErrorCount(), equalTo(1L)); assertThat(delegate.getStatistics().getLoadCount(), equalTo(1L)); assertThat(delegate.getStatistics().getBatchLoadCount(), equalTo(1L)); assertThat(delegate.getStatistics().getCacheHitCount(), equalTo(1L)); assertThat(delegate.getStatistics().getCacheMissCount(), equalTo(0L)); + assertThat(delegate.getStatistics().getBatchLoadExceptionCount(), equalTo(1L)); + assertThat(delegate.getStatistics().getLoadErrorCount(), equalTo(1L)); } } \ No newline at end of file From 8d0376bcc06a71f5cad8d3a5bfa9e96a5f4c172d Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 19 Sep 2017 17:24:12 -0700 Subject: [PATCH 4/9] Added noop version of collector --- .../stats/NoOpStatisticsCollector.java | 39 +++++++++++++++++++ .../stats/StatisticsCollectorTest.java | 14 +++++++ 2 files changed, 53 insertions(+) create mode 100644 src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java diff --git a/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java new file mode 100644 index 0000000..0d6432a --- /dev/null +++ b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java @@ -0,0 +1,39 @@ +package org.dataloader.stats; + +/** + * A statistics collector that does nothing + */ +public class NoOpStatisticsCollector implements StatisticsCollector { + + private static final StatisticsImpl ZERO_STATS = new StatisticsImpl(0, 0, 0, 0, 0); + + @Override + public long incrementLoadCount() { + return 0; + } + + @Override + public long incrementLoadErrorCount() { + return 0; + } + + @Override + public long incrementBatchLoadCount() { + return 0; + } + + @Override + public long incrementBatchLoadExceptionCount() { + return 0; + } + + @Override + public long incrementCacheHitCount() { + return 0; + } + + @Override + public Statistics getStatistics() { + return ZERO_STATS; + } +} diff --git a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java index ba98ddc..d934c8e 100644 --- a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java +++ b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java @@ -194,6 +194,20 @@ public void delegating_collector_works() throws Exception { assertThat(delegate.getStatistics().getCacheMissCount(), equalTo(0L)); assertThat(delegate.getStatistics().getBatchLoadExceptionCount(), equalTo(1L)); assertThat(delegate.getStatistics().getLoadErrorCount(), equalTo(1L)); + } + + @Test + public void noop_is_just_that() throws Exception { + StatisticsCollector collector = new NoOpStatisticsCollector(); + collector.incrementLoadErrorCount(); + collector.incrementBatchLoadExceptionCount(); + collector.incrementBatchLoadCount(); + collector.incrementCacheHitCount(); + + assertThat(collector.getStatistics().getLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getBatchLoadCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheHitCount(), equalTo(0L)); + assertThat(collector.getStatistics().getCacheMissCount(), equalTo(0L)); } } \ No newline at end of file From 86aa043036c902b2ee0b3114dcc7f20c8474bcc4 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 19 Sep 2017 19:44:02 -0700 Subject: [PATCH 5/9] Made it doubles and added toMap --- .../org/dataloader/DataLoaderRegistry.java | 15 ++++ .../stats/SimpleStatisticsCollector.java | 2 +- .../java/org/dataloader/stats/Statistics.java | 24 +++++- .../org/dataloader/stats/StatisticsImpl.java | 80 ++++++++++++++----- .../dataloader/DataLoaderRegistryTest.java | 33 ++++++++ .../stats/StatisticsCollectorTest.java | 12 +-- .../dataloader/stats/StatisticsImplTest.java | 43 ++++++++++ 7 files changed, 177 insertions(+), 32 deletions(-) create mode 100644 src/test/java/org/dataloader/stats/StatisticsImplTest.java diff --git a/src/main/java/org/dataloader/DataLoaderRegistry.java b/src/main/java/org/dataloader/DataLoaderRegistry.java index 2c11d4c..9df6fc2 100644 --- a/src/main/java/org/dataloader/DataLoaderRegistry.java +++ b/src/main/java/org/dataloader/DataLoaderRegistry.java @@ -1,5 +1,8 @@ package org.dataloader; +import org.dataloader.stats.Statistics; +import org.dataloader.stats.StatisticsImpl; + import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -91,4 +94,16 @@ public Set getKeys() { public void dispatchAll() { getDataLoaders().forEach(DataLoader::dispatch); } + + /** + * @return a combined set of statistics for all data loaders in this registry presented + * as the sum of all their statistics + */ + public Statistics getStatistics() { + Statistics stats = new StatisticsImpl(); + for (DataLoader dataLoader : dataLoaders.values()) { + stats = stats.combine(dataLoader.getStatistics()); + } + return stats; + } } diff --git a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java index 3d78739..888f5f0 100644 --- a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java @@ -42,7 +42,7 @@ public long incrementBatchLoadExceptionCount() { @Override public Statistics getStatistics() { - return new StatisticsImpl(loadCount.get(), batchLoadCount.get(), cacheHitCount.get(), batchLoadExceptionCount.get(), loadErrorCount.get()); + return new StatisticsImpl(loadCount.get(), loadErrorCount.get(), batchLoadCount.get(), batchLoadExceptionCount.get(), cacheHitCount.get()); } @Override diff --git a/src/main/java/org/dataloader/stats/Statistics.java b/src/main/java/org/dataloader/stats/Statistics.java index 5f9e56d..e906613 100644 --- a/src/main/java/org/dataloader/stats/Statistics.java +++ b/src/main/java/org/dataloader/stats/Statistics.java @@ -1,5 +1,7 @@ package org.dataloader.stats; +import java.util.Map; + /** * A simple set of statistics about {@link org.dataloader.DataLoader} operations */ @@ -17,7 +19,7 @@ public interface Statistics { /** * @return batchLoadCount / loadCount */ - float getBatchLoadRatio(); + double getBatchLoadRatio(); /** * @return the number of times {@link org.dataloader.DataLoader#load(Object)} resulted in a cache hit @@ -32,7 +34,7 @@ public interface Statistics { /** * @return cacheHits / loadCount */ - float getCacheHitRatio(); + double getCacheHitRatio(); /** * @return the number of times the {@link org.dataloader.DataLoader} batch loader function throw an exception when trying to get any values @@ -42,7 +44,7 @@ public interface Statistics { /** * @return batchLoadExceptionCount / loadCount */ - float getBatchLoadExceptionRatio(); + double getBatchLoadExceptionRatio(); /** * @return the number of times the {@link org.dataloader.DataLoader} batch loader function return an specific object that was in error @@ -53,6 +55,20 @@ public interface Statistics { /** * @return loadErrorCount / loadCount */ - float getLoadErrorRatio(); + double getLoadErrorRatio(); + + /** + * This will combine this set of statistics with another set of statistics so that they become the combined count of each + * + * @param statistics the other statistics to combine + * + * @return a new statistics object of the combined counts + */ + Statistics combine(Statistics statistics); + + /** + * @return a map representation of the statistics, perhaps to send over JSON or some such + */ + Map toMap(); } diff --git a/src/main/java/org/dataloader/stats/StatisticsImpl.java b/src/main/java/org/dataloader/stats/StatisticsImpl.java index 4c2ac18..3c78851 100644 --- a/src/main/java/org/dataloader/stats/StatisticsImpl.java +++ b/src/main/java/org/dataloader/stats/StatisticsImpl.java @@ -1,14 +1,24 @@ package org.dataloader.stats; +import java.util.LinkedHashMap; +import java.util.Map; + public class StatisticsImpl implements Statistics { private final long loadCount; + private final long loadErrorCount; private final long batchLoadCount; - private final long cacheHitCount; private final long batchLoadExceptionCount; - private final long loadErrorCount; + private final long cacheHitCount; - public StatisticsImpl(long loadCount, long batchLoadCount, long cacheHitCount, long batchLoadExceptionCount, long loadErrorCount) { + /** + * Zero statistics + */ + public StatisticsImpl() { + this(0, 0, 0, 0, 0); + } + + public StatisticsImpl(long loadCount, long loadErrorCount, long batchLoadCount, long batchLoadExceptionCount, long cacheHitCount) { this.loadCount = loadCount; this.batchLoadCount = batchLoadCount; this.cacheHitCount = cacheHitCount; @@ -16,23 +26,44 @@ public StatisticsImpl(long loadCount, long batchLoadCount, long cacheHitCount, l this.loadErrorCount = loadErrorCount; } + private double ratio(long numerator, long denominator) { + return denominator == 0 ? 0f : ((double) numerator) / ((double) denominator); + } + @Override public long getLoadCount() { return loadCount; } + + @Override + public long getLoadErrorCount() { + return loadErrorCount; + } + + @Override + public double getLoadErrorRatio() { + return ratio(loadErrorCount, loadCount); + } + @Override public long getBatchLoadCount() { return batchLoadCount; } @Override - public float getBatchLoadRatio() { + public double getBatchLoadRatio() { return ratio(batchLoadCount, loadCount); } - private float ratio(long numerator, long denominator) { - return denominator == 0 ? 0f : ((float) numerator) / ((float) denominator); + @Override + public long getBatchLoadExceptionCount() { + return batchLoadExceptionCount; + } + + @Override + public double getBatchLoadExceptionRatio() { + return ratio(batchLoadExceptionCount, loadCount); } @Override @@ -46,38 +77,45 @@ public long getCacheMissCount() { } @Override - public float getCacheHitRatio() { + public double getCacheHitRatio() { return ratio(cacheHitCount, loadCount); } - @Override - public long getBatchLoadExceptionCount() { - return batchLoadExceptionCount; - } @Override - public long getLoadErrorCount() { - return loadErrorCount; + public Statistics combine(Statistics other) { + return new StatisticsImpl( + this.loadCount + other.getLoadCount(), + this.loadErrorCount + other.getLoadErrorCount(), this.batchLoadCount + other.getBatchLoadCount(), + this.batchLoadExceptionCount + other.getBatchLoadExceptionCount(), this.cacheHitCount + other.getCacheHitCount() + ); } @Override - public float getBatchLoadExceptionRatio() { - return ratio(batchLoadExceptionCount, loadCount); - } + public Map toMap() { + Map stats = new LinkedHashMap<>(); + stats.put("loadCount", getLoadCount()); + stats.put("loadErrorCount", getLoadErrorCount()); + stats.put("loadErrorRatio", getLoadErrorRatio()); - @Override - public float getLoadErrorRatio() { - return ratio(loadErrorCount, loadCount); + stats.put("batchLoadCount", getBatchLoadCount()); + stats.put("batchLoadRatio", getBatchLoadRatio()); + stats.put("batchLoadExceptionCount", getBatchLoadExceptionCount()); + stats.put("batchLoadExceptionRatio", getBatchLoadExceptionRatio()); + + stats.put("cacheHitCount", getCacheHitCount()); + stats.put("cacheHitRatio", getCacheHitRatio()); + return stats; } @Override public String toString() { return "StatisticsImpl{" + "loadCount=" + loadCount + + ", loadErrorCount=" + loadErrorCount + ", batchLoadCount=" + batchLoadCount + - ", cacheHitCount=" + cacheHitCount + ", batchLoadExceptionCount=" + batchLoadExceptionCount + - ", loadErrorCount=" + loadErrorCount + + ", cacheHitCount=" + cacheHitCount + '}'; } } diff --git a/src/test/java/org/dataloader/DataLoaderRegistryTest.java b/src/test/java/org/dataloader/DataLoaderRegistryTest.java index 1e1c035..aa6a84f 100644 --- a/src/test/java/org/dataloader/DataLoaderRegistryTest.java +++ b/src/test/java/org/dataloader/DataLoaderRegistryTest.java @@ -1,5 +1,6 @@ package org.dataloader; +import org.dataloader.stats.Statistics; import org.junit.Test; import java.util.concurrent.CompletableFuture; @@ -68,4 +69,36 @@ public void registries_can_be_combined() throws Exception { assertThat(combinedRegistry.getKeys(), hasItems("a", "b", "c", "d")); assertThat(combinedRegistry.getDataLoaders(), hasItems(dlA, dlB, dlC, dlD)); } + + @Test + public void stats_can_be_collected() throws Exception { + + DataLoaderRegistry registry = new DataLoaderRegistry(); + + DataLoader dlA = new DataLoader<>(identityBatchLoader); + DataLoader dlB = new DataLoader<>(identityBatchLoader); + DataLoader dlC = new DataLoader<>(identityBatchLoader); + + registry.register("a", dlA).register("b", dlB).register("c", dlC); + + dlA.load("X"); + dlB.load("Y"); + dlC.load("Z"); + + registry.dispatchAll(); + + dlA.load("X"); + dlB.load("Y"); + dlC.load("Z"); + + registry.dispatchAll(); + + Statistics statistics = registry.getStatistics(); + + assertThat(statistics.getLoadCount(), equalTo(6L)); + assertThat(statistics.getBatchLoadCount(), equalTo(3L)); + assertThat(statistics.getCacheHitCount(), equalTo(3L)); + assertThat(statistics.getLoadErrorCount(), equalTo(0L)); + assertThat(statistics.getBatchLoadExceptionCount(), equalTo(0L)); + } } \ No newline at end of file diff --git a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java index d934c8e..21519e3 100644 --- a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java +++ b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java @@ -43,8 +43,8 @@ public void ratios_work() throws Exception { collector.incrementLoadCount(); Statistics stats = collector.getStatistics(); - assertThat(stats.getBatchLoadRatio(), equalTo(0f)); - assertThat(stats.getCacheHitRatio(), equalTo(0f)); + assertThat(stats.getBatchLoadRatio(), equalTo(0d)); + assertThat(stats.getCacheHitRatio(), equalTo(0d)); collector.incrementLoadCount(); @@ -53,7 +53,7 @@ public void ratios_work() throws Exception { collector.incrementBatchLoadCount(); stats = collector.getStatistics(); - assertThat(stats.getBatchLoadRatio(), equalTo(1f / 4f)); + assertThat(stats.getBatchLoadRatio(), equalTo(1d / 4d)); collector.incrementLoadCount(); @@ -63,7 +63,7 @@ public void ratios_work() throws Exception { collector.incrementCacheHitCount(); stats = collector.getStatistics(); - assertThat(stats.getCacheHitRatio(), equalTo(2f / 7f)); + assertThat(stats.getCacheHitRatio(), equalTo(2d / 7d)); collector.incrementLoadCount(); collector.incrementLoadCount(); @@ -72,7 +72,7 @@ public void ratios_work() throws Exception { collector.incrementBatchLoadExceptionCount(); stats = collector.getStatistics(); - assertThat(stats.getBatchLoadExceptionRatio(), equalTo(2f / 10f)); + assertThat(stats.getBatchLoadExceptionRatio(), equalTo(2d / 10d)); collector.incrementLoadCount(); collector.incrementLoadCount(); @@ -82,7 +82,7 @@ public void ratios_work() throws Exception { collector.incrementLoadErrorCount(); stats = collector.getStatistics(); - assertThat(stats.getLoadErrorRatio(), equalTo(3f / 13f)); + assertThat(stats.getLoadErrorRatio(), equalTo(3d / 13d)); } @Test diff --git a/src/test/java/org/dataloader/stats/StatisticsImplTest.java b/src/test/java/org/dataloader/stats/StatisticsImplTest.java new file mode 100644 index 0000000..a4d330d --- /dev/null +++ b/src/test/java/org/dataloader/stats/StatisticsImplTest.java @@ -0,0 +1,43 @@ +package org.dataloader.stats; + +import org.junit.Test; + +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +public class StatisticsImplTest { + + @Test + public void combine_works() throws Exception { + StatisticsImpl one = new StatisticsImpl(1, 5, 2, 4, 3); + StatisticsImpl two = new StatisticsImpl(6, 10, 7, 9, 8); + + Statistics combine = one.combine(two); + + assertThat(combine.getLoadCount(), equalTo(7L)); + assertThat(combine.getBatchLoadCount(), equalTo(9L)); + assertThat(combine.getCacheHitCount(), equalTo(11L)); + assertThat(combine.getBatchLoadExceptionCount(), equalTo(13L)); + assertThat(combine.getLoadErrorCount(), equalTo(15L)); + } + + @Test + public void to_map_works() throws Exception { + + StatisticsImpl one = new StatisticsImpl(10, 2, 3, 4, 5); + Map map = one.toMap(); + + assertThat(map.get("loadCount"), equalTo(10L)); + assertThat(map.get("loadErrorCount"), equalTo(2L)); + assertThat(map.get("loadErrorRatio"), equalTo(0.2d)); + assertThat(map.get("batchLoadCount"), equalTo(3L)); + assertThat(map.get("batchLoadRatio"), equalTo(0.3d)); + assertThat(map.get("batchLoadExceptionCount"), equalTo(4L)); + assertThat(map.get("batchLoadExceptionRatio"), equalTo(0.4d)); + assertThat(map.get("cacheHitCount"), equalTo(5L)); + assertThat(map.get("cacheHitRatio"), equalTo(0.5d)); + + } +} \ No newline at end of file From 930eafe4c881341cdd22a3d9d8df4fe827b4bf41 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 19 Sep 2017 20:13:30 -0700 Subject: [PATCH 6/9] javadoc warning --- src/main/java/org/dataloader/DataLoaderOptions.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/dataloader/DataLoaderOptions.java b/src/main/java/org/dataloader/DataLoaderOptions.java index 8d55f84..97b19ad 100644 --- a/src/main/java/org/dataloader/DataLoaderOptions.java +++ b/src/main/java/org/dataloader/DataLoaderOptions.java @@ -193,6 +193,8 @@ public StatisticsCollector getStatisticsCollector() { * the supplier pattern, you can create a new statistics collector on each call or you can reuse * a common value * + * @param statisticsCollector the statistics collector to use + * * @return the data loader options for fluent coding */ public DataLoaderOptions setStatisticsCollector(Supplier statisticsCollector) { From 5b5b1b819363377e0cf85f18b8f6cbbe6842e657 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Tue, 19 Sep 2017 23:04:26 -0700 Subject: [PATCH 7/9] batch load count is now the number of objects loaded via the batch function and batch invoke count the number of invocations of the batch function --- src/main/java/org/dataloader/DataLoader.java | 4 +-- .../stats/DelegatingStatisticsCollector.java | 6 ++-- .../stats/NoOpStatisticsCollector.java | 4 +-- .../stats/SimpleStatisticsCollector.java | 9 ++++-- .../java/org/dataloader/stats/Statistics.java | 7 +++- .../dataloader/stats/StatisticsCollector.java | 4 ++- .../org/dataloader/stats/StatisticsImpl.java | 19 ++++++++--- .../stats/ThreadLocalStatisticsCollector.java | 6 ++-- .../org/dataloader/DataLoaderStatsTest.java | 32 ++++++++++++------- .../stats/StatisticsCollectorTest.java | 14 ++++---- .../dataloader/stats/StatisticsImplTest.java | 8 +++-- 11 files changed, 73 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/dataloader/DataLoader.java b/src/main/java/org/dataloader/DataLoader.java index cf07a8c..a3fdc2b 100644 --- a/src/main/java/org/dataloader/DataLoader.java +++ b/src/main/java/org/dataloader/DataLoader.java @@ -193,13 +193,13 @@ public CompletableFuture load(K key) { loaderQueue.put(key, future); } } else { + stats.incrementBatchLoadCountBy(1); // immediate execution of batch function CompletableFuture> batchedLoad = batchLoadFunction .load(singletonList(key)) .toCompletableFuture(); future = batchedLoad .thenApply(list -> list.get(0)); - stats.incrementBatchLoadCount(); } if (loaderOptions.cachingEnabled()) { synchronized (futureCache) { @@ -300,7 +300,7 @@ private CompletableFuture> sliceIntoBatchesOfBatches(List keys, List< @SuppressWarnings("unchecked") private CompletableFuture> dispatchQueueBatch(List keys, List> queuedFutures) { - stats.incrementBatchLoadCount(); + stats.incrementBatchLoadCountBy(keys.size()); CompletionStage> batchLoad; try { batchLoad = nonNull(batchLoadFunction.load(keys), "Your batch loader function MUST return a non null CompletionStage promise"); diff --git a/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java index 5b74d5c..f44c521 100644 --- a/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/DelegatingStatisticsCollector.java @@ -26,9 +26,9 @@ public long incrementLoadCount() { } @Override - public long incrementBatchLoadCount() { - delegateCollector.incrementBatchLoadCount(); - return collector.incrementBatchLoadCount(); + public long incrementBatchLoadCountBy(long delta) { + delegateCollector.incrementBatchLoadCountBy(delta); + return collector.incrementBatchLoadCountBy(delta); } @Override diff --git a/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java index 0d6432a..bd18bea 100644 --- a/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java @@ -5,7 +5,7 @@ */ public class NoOpStatisticsCollector implements StatisticsCollector { - private static final StatisticsImpl ZERO_STATS = new StatisticsImpl(0, 0, 0, 0, 0); + private static final StatisticsImpl ZERO_STATS = new StatisticsImpl(); @Override public long incrementLoadCount() { @@ -18,7 +18,7 @@ public long incrementLoadErrorCount() { } @Override - public long incrementBatchLoadCount() { + public long incrementBatchLoadCountBy(long delta) { return 0; } diff --git a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java index 888f5f0..7e5b89d 100644 --- a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java @@ -10,6 +10,7 @@ */ public class SimpleStatisticsCollector implements StatisticsCollector { private final AtomicLong loadCount = new AtomicLong(); + private final AtomicLong batchInvokeCount = new AtomicLong(); private final AtomicLong batchLoadCount = new AtomicLong(); private final AtomicLong cacheHitCount = new AtomicLong(); private final AtomicLong batchLoadExceptionCount = new AtomicLong(); @@ -20,9 +21,11 @@ public long incrementLoadCount() { return loadCount.incrementAndGet(); } + @Override - public long incrementBatchLoadCount() { - return batchLoadCount.incrementAndGet(); + public long incrementBatchLoadCountBy(long delta) { + batchInvokeCount.incrementAndGet(); + return batchLoadCount.addAndGet(delta); } @Override @@ -42,7 +45,7 @@ public long incrementBatchLoadExceptionCount() { @Override public Statistics getStatistics() { - return new StatisticsImpl(loadCount.get(), loadErrorCount.get(), batchLoadCount.get(), batchLoadExceptionCount.get(), cacheHitCount.get()); + return new StatisticsImpl(loadCount.get(), loadErrorCount.get(), batchInvokeCount.get(), batchLoadCount.get(), batchLoadExceptionCount.get(), cacheHitCount.get()); } @Override diff --git a/src/main/java/org/dataloader/stats/Statistics.java b/src/main/java/org/dataloader/stats/Statistics.java index e906613..58a4acc 100644 --- a/src/main/java/org/dataloader/stats/Statistics.java +++ b/src/main/java/org/dataloader/stats/Statistics.java @@ -7,13 +7,18 @@ */ public interface Statistics { /** - * @return the number of times {@link org.dataloader.DataLoader#load(Object)} has been called + * @return the number of objects {@link org.dataloader.DataLoader#load(Object)} has been asked to load */ long getLoadCount(); /** * @return the number of times the {@link org.dataloader.DataLoader} batch loader function has been called */ + long getBatchInvokeCount(); + + /** + * @return the number of objects that the {@link org.dataloader.DataLoader} batch loader function has been asked to load + */ long getBatchLoadCount(); /** diff --git a/src/main/java/org/dataloader/stats/StatisticsCollector.java b/src/main/java/org/dataloader/stats/StatisticsCollector.java index fcec15e..874e27b 100644 --- a/src/main/java/org/dataloader/stats/StatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/StatisticsCollector.java @@ -22,9 +22,11 @@ public interface StatisticsCollector { /** * Called to increment the number of batch loads * + * @param delta how much to add to the count + * * @return the current value after increment */ - long incrementBatchLoadCount(); + long incrementBatchLoadCountBy(long delta); /** * Called to increment the number of batch loads exceptions diff --git a/src/main/java/org/dataloader/stats/StatisticsImpl.java b/src/main/java/org/dataloader/stats/StatisticsImpl.java index 3c78851..b6442b5 100644 --- a/src/main/java/org/dataloader/stats/StatisticsImpl.java +++ b/src/main/java/org/dataloader/stats/StatisticsImpl.java @@ -7,6 +7,7 @@ public class StatisticsImpl implements Statistics { private final long loadCount; private final long loadErrorCount; + private final long batchInvokeCount; private final long batchLoadCount; private final long batchLoadExceptionCount; private final long cacheHitCount; @@ -15,11 +16,12 @@ public class StatisticsImpl implements Statistics { * Zero statistics */ public StatisticsImpl() { - this(0, 0, 0, 0, 0); + this(0, 0, 0, 0, 0, 0); } - public StatisticsImpl(long loadCount, long loadErrorCount, long batchLoadCount, long batchLoadExceptionCount, long cacheHitCount) { + public StatisticsImpl(long loadCount, long loadErrorCount, long batchInvokeCount, long batchLoadCount, long batchLoadExceptionCount, long cacheHitCount) { this.loadCount = loadCount; + this.batchInvokeCount = batchInvokeCount; this.batchLoadCount = batchLoadCount; this.cacheHitCount = cacheHitCount; this.batchLoadExceptionCount = batchLoadExceptionCount; @@ -46,6 +48,11 @@ public double getLoadErrorRatio() { return ratio(loadErrorCount, loadCount); } + @Override + public long getBatchInvokeCount() { + return batchInvokeCount; + } + @Override public long getBatchLoadCount() { return batchLoadCount; @@ -86,8 +93,11 @@ public double getCacheHitRatio() { public Statistics combine(Statistics other) { return new StatisticsImpl( this.loadCount + other.getLoadCount(), - this.loadErrorCount + other.getLoadErrorCount(), this.batchLoadCount + other.getBatchLoadCount(), - this.batchLoadExceptionCount + other.getBatchLoadExceptionCount(), this.cacheHitCount + other.getCacheHitCount() + this.loadErrorCount + other.getLoadErrorCount(), + this.batchInvokeCount + other.getBatchInvokeCount(), + this.batchLoadCount + other.getBatchLoadCount(), + this.batchLoadExceptionCount + other.getBatchLoadExceptionCount(), + this.cacheHitCount + other.getCacheHitCount() ); } @@ -98,6 +108,7 @@ public Map toMap() { stats.put("loadErrorCount", getLoadErrorCount()); stats.put("loadErrorRatio", getLoadErrorRatio()); + stats.put("batchInvokeCount", getBatchInvokeCount()); stats.put("batchLoadCount", getBatchLoadCount()); stats.put("batchLoadRatio", getBatchLoadRatio()); stats.put("batchLoadExceptionCount", getBatchLoadExceptionCount()); diff --git a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java index faacf18..7093584 100644 --- a/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/ThreadLocalStatisticsCollector.java @@ -35,9 +35,9 @@ public long incrementLoadCount() { } @Override - public long incrementBatchLoadCount() { - overallCollector.incrementBatchLoadCount(); - return collector.get().incrementBatchLoadCount(); + public long incrementBatchLoadCountBy(long delta) { + overallCollector.incrementBatchLoadCountBy(delta); + return collector.get().incrementBatchLoadCountBy(delta); } @Override diff --git a/src/test/java/org/dataloader/DataLoaderStatsTest.java b/src/test/java/org/dataloader/DataLoaderStatsTest.java index a7f3101..c6a355b 100644 --- a/src/test/java/org/dataloader/DataLoaderStatsTest.java +++ b/src/test/java/org/dataloader/DataLoaderStatsTest.java @@ -31,6 +31,7 @@ public void stats_are_collected_by_default() throws Exception { Statistics stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchInvokeCount(), equalTo(0L)); assertThat(stats.getBatchLoadCount(), equalTo(0L)); assertThat(stats.getCacheHitCount(), equalTo(0L)); @@ -38,7 +39,8 @@ public void stats_are_collected_by_default() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(4L)); - assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getBatchInvokeCount(), equalTo(1L)); + assertThat(stats.getBatchLoadCount(), equalTo(4L)); assertThat(stats.getCacheHitCount(), equalTo(0L)); loader.load("A"); @@ -48,7 +50,8 @@ public void stats_are_collected_by_default() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(6L)); - assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getBatchInvokeCount(), equalTo(1L)); + assertThat(stats.getBatchLoadCount(), equalTo(4L)); assertThat(stats.getCacheHitCount(), equalTo(2L)); } @@ -58,7 +61,7 @@ public void stats_are_collected_with_specified_collector() throws Exception { // lets prime it with some numbers so we know its ours StatisticsCollector collector = new SimpleStatisticsCollector(); collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); BatchLoader batchLoader = CompletableFuture::completedFuture; DataLoaderOptions loaderOptions = DataLoaderOptions.newOptions().setStatisticsCollector(() -> collector); @@ -70,14 +73,16 @@ public void stats_are_collected_with_specified_collector() throws Exception { Statistics stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(5L)); // previously primed with 1 - assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getBatchInvokeCount(), equalTo(1L)); // also primed + assertThat(stats.getBatchLoadCount(), equalTo(1L)); // also primed assertThat(stats.getCacheHitCount(), equalTo(0L)); loader.dispatch(); stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(5L)); - assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getBatchInvokeCount(), equalTo(2L)); + assertThat(stats.getBatchLoadCount(), equalTo(5L)); assertThat(stats.getCacheHitCount(), equalTo(0L)); loader.load("A"); @@ -87,7 +92,8 @@ public void stats_are_collected_with_specified_collector() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(7L)); - assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getBatchInvokeCount(), equalTo(2L)); + assertThat(stats.getBatchLoadCount(), equalTo(5L)); assertThat(stats.getCacheHitCount(), equalTo(2L)); } @@ -105,6 +111,7 @@ public void stats_are_collected_with_caching_disabled() throws Exception { Statistics stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(4L)); + assertThat(stats.getBatchInvokeCount(), equalTo(0L)); assertThat(stats.getBatchLoadCount(), equalTo(0L)); assertThat(stats.getCacheHitCount(), equalTo(0L)); @@ -112,7 +119,8 @@ public void stats_are_collected_with_caching_disabled() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(4L)); - assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getBatchInvokeCount(), equalTo(1L)); + assertThat(stats.getBatchLoadCount(), equalTo(4L)); assertThat(stats.getCacheHitCount(), equalTo(0L)); loader.load("A"); @@ -122,7 +130,8 @@ public void stats_are_collected_with_caching_disabled() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(6L)); - assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getBatchInvokeCount(), equalTo(2L)); + assertThat(stats.getBatchLoadCount(), equalTo(6L)); assertThat(stats.getCacheHitCount(), equalTo(0L)); } @@ -158,7 +167,8 @@ public void stats_are_collected_on_exceptions() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(2L)); - assertThat(stats.getBatchLoadCount(), equalTo(1L)); + assertThat(stats.getBatchInvokeCount(), equalTo(1L)); + assertThat(stats.getBatchLoadCount(), equalTo(2L)); assertThat(stats.getBatchLoadExceptionCount(), equalTo(1L)); assertThat(stats.getLoadErrorCount(), equalTo(0L)); @@ -170,7 +180,7 @@ public void stats_are_collected_on_exceptions() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(5L)); - assertThat(stats.getBatchLoadCount(), equalTo(2L)); + assertThat(stats.getBatchLoadCount(), equalTo(5L)); assertThat(stats.getBatchLoadExceptionCount(), equalTo(1L)); assertThat(stats.getLoadErrorCount(), equalTo(3L)); @@ -180,7 +190,7 @@ public void stats_are_collected_on_exceptions() throws Exception { stats = loader.getStatistics(); assertThat(stats.getLoadCount(), equalTo(6L)); - assertThat(stats.getBatchLoadCount(), equalTo(3L)); + assertThat(stats.getBatchLoadCount(), equalTo(6L)); assertThat(stats.getBatchLoadExceptionCount(), equalTo(2L)); assertThat(stats.getLoadErrorCount(), equalTo(3L)); } diff --git a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java index 21519e3..2b5f5df 100644 --- a/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java +++ b/src/test/java/org/dataloader/stats/StatisticsCollectorTest.java @@ -22,7 +22,7 @@ public void basic_collection() throws Exception { collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); collector.incrementCacheHitCount(); collector.incrementBatchLoadExceptionCount(); collector.incrementLoadErrorCount(); @@ -50,7 +50,7 @@ public void ratios_work() throws Exception { collector.incrementLoadCount(); collector.incrementLoadCount(); collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); stats = collector.getStatistics(); assertThat(stats.getBatchLoadRatio(), equalTo(1d / 4d)); @@ -96,7 +96,7 @@ public void thread_local_collection() throws Exception { collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); collector.incrementCacheHitCount(); assertThat(collector.getStatistics().getLoadCount(), equalTo(1L)); @@ -110,7 +110,7 @@ public void thread_local_collection() throws Exception { CompletableFuture.supplyAsync(() -> { collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); collector.incrementCacheHitCount(); // per thread stats here @@ -129,7 +129,7 @@ public void thread_local_collection() throws Exception { // back on this main thread collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); collector.incrementCacheHitCount(); // per thread stats here @@ -169,7 +169,7 @@ public void delegating_collector_works() throws Exception { collector.incrementLoadCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); collector.incrementCacheHitCount(); collector.incrementBatchLoadExceptionCount(); collector.incrementLoadErrorCount(); @@ -201,7 +201,7 @@ public void noop_is_just_that() throws Exception { StatisticsCollector collector = new NoOpStatisticsCollector(); collector.incrementLoadErrorCount(); collector.incrementBatchLoadExceptionCount(); - collector.incrementBatchLoadCount(); + collector.incrementBatchLoadCountBy(1); collector.incrementCacheHitCount(); assertThat(collector.getStatistics().getLoadCount(), equalTo(0L)); diff --git a/src/test/java/org/dataloader/stats/StatisticsImplTest.java b/src/test/java/org/dataloader/stats/StatisticsImplTest.java index a4d330d..87adebe 100644 --- a/src/test/java/org/dataloader/stats/StatisticsImplTest.java +++ b/src/test/java/org/dataloader/stats/StatisticsImplTest.java @@ -11,8 +11,8 @@ public class StatisticsImplTest { @Test public void combine_works() throws Exception { - StatisticsImpl one = new StatisticsImpl(1, 5, 2, 4, 3); - StatisticsImpl two = new StatisticsImpl(6, 10, 7, 9, 8); + StatisticsImpl one = new StatisticsImpl(1, 5, 6, 2, 4, 3); + StatisticsImpl two = new StatisticsImpl(6, 10, 6, 7, 9, 8); Statistics combine = one.combine(two); @@ -21,17 +21,19 @@ public void combine_works() throws Exception { assertThat(combine.getCacheHitCount(), equalTo(11L)); assertThat(combine.getBatchLoadExceptionCount(), equalTo(13L)); assertThat(combine.getLoadErrorCount(), equalTo(15L)); + assertThat(combine.getBatchInvokeCount(), equalTo(12L)); } @Test public void to_map_works() throws Exception { - StatisticsImpl one = new StatisticsImpl(10, 2, 3, 4, 5); + StatisticsImpl one = new StatisticsImpl(10, 2, 11, 3, 4, 5); Map map = one.toMap(); assertThat(map.get("loadCount"), equalTo(10L)); assertThat(map.get("loadErrorCount"), equalTo(2L)); assertThat(map.get("loadErrorRatio"), equalTo(0.2d)); + assertThat(map.get("batchInvokeCount"), equalTo(11L)); assertThat(map.get("batchLoadCount"), equalTo(3L)); assertThat(map.get("batchLoadRatio"), equalTo(0.3d)); assertThat(map.get("batchLoadExceptionCount"), equalTo(4L)); From 165b8ac678adc22de93c00446a641281c38d7ab5 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 20 Sep 2017 11:11:48 -0700 Subject: [PATCH 8/9] readme updates --- README.md | 32 +++++++++++++++++++++++++++++++ src/test/java/ReadmeExamples.java | 20 ++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4d19bad..293d0ba 100644 --- a/README.md +++ b/README.md @@ -269,6 +269,38 @@ In some circumstances you may wish to clear the cache for these individual probl }); ``` + +## Statistics on what is happening + +`DataLoader` keeps statistics on what is happening. It can tell you the number of objects asked for, the cache hit number, the number of objects +asked for via batching and so on. + +Knowing what the behaviour of your data is important for you to understand how efficient you are in serving the data via this pattern. + + +```java + Statistics statistics = userDataLoader.getStatistics(); + + System.out.println(format("load : %d", statistics.getLoadCount())); + System.out.println(format("batch load: %d", statistics.getBatchLoadCount())); + System.out.println(format("cache hit: %d", statistics.getCacheHitCount())); + System.out.println(format("cache hit ratio: %d", statistics.getCacheHitRatio())); + +``` + +`DataLoaderRegistry` can also roll up the statistics for all data loaders inside it. + +You can configure the statistics collector used when you build the data loader + +```java + DataLoaderOptions options = DataLoaderOptions.newOptions().setStatisticsCollector(() -> new ThreadLocalStatisticsCollector()); + DataLoader userDataLoader = DataLoader.newDataLoader(userBatchLoader,options); + +``` + +Which collector you use is up to you. It ships with the following: `SimpleStatisticsCollector`, `ThreadLocalStatisticsCollector`, `DelegatingStatisticsCollector` +and `NoOpStatisticsCollector`. + ## The scope of a data loader is important If you are serving web requests then the data can be specific to the user requesting it. If you have user specific data diff --git a/src/test/java/ReadmeExamples.java b/src/test/java/ReadmeExamples.java index fe94f23..1fa722e 100644 --- a/src/test/java/ReadmeExamples.java +++ b/src/test/java/ReadmeExamples.java @@ -5,6 +5,8 @@ import org.dataloader.Try; import org.dataloader.fixtures.User; import org.dataloader.fixtures.UserManager; +import org.dataloader.stats.Statistics; +import org.dataloader.stats.ThreadLocalStatisticsCollector; import java.util.ArrayList; import java.util.List; @@ -12,6 +14,8 @@ import java.util.concurrent.CompletionStage; import java.util.stream.Collectors; +import static java.lang.String.format; + @SuppressWarnings("ALL") public class ReadmeExamples { @@ -75,7 +79,6 @@ public CompletionStage> load(List userIds) { } - private void tryExample() { Try tryS = Try.tryCall(() -> { if (rollDice()) { @@ -185,4 +188,19 @@ private boolean rollDice() { } + private void statsExample() { + Statistics statistics = userDataLoader.getStatistics(); + + System.out.println(format("load : %d", statistics.getLoadCount())); + System.out.println(format("batch load: %d", statistics.getBatchLoadCount())); + System.out.println(format("cache hit: %d", statistics.getCacheHitCount())); + System.out.println(format("cache hit ratio: %d", statistics.getCacheHitRatio())); + } + + private void statsConfigExample() { + + DataLoaderOptions options = DataLoaderOptions.newOptions().setStatisticsCollector(() -> new ThreadLocalStatisticsCollector()); + DataLoader userDataLoader = DataLoader.newDataLoader(userBatchLoader,options); + } + } From e684db52afa6ae3f7ffdc23701c0db3b281346ba Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 20 Sep 2017 11:46:08 -0700 Subject: [PATCH 9/9] Made Statistics a class --- .../org/dataloader/DataLoaderRegistry.java | 3 +- .../stats/NoOpStatisticsCollector.java | 2 +- .../stats/SimpleStatisticsCollector.java | 2 +- .../java/org/dataloader/stats/Statistics.java | 143 +++++++++++++++--- .../org/dataloader/stats/StatisticsImpl.java | 132 ---------------- ...sticsImplTest.java => StatisticsTest.java} | 8 +- 6 files changed, 125 insertions(+), 165 deletions(-) delete mode 100644 src/main/java/org/dataloader/stats/StatisticsImpl.java rename src/test/java/org/dataloader/stats/{StatisticsImplTest.java => StatisticsTest.java} (85%) diff --git a/src/main/java/org/dataloader/DataLoaderRegistry.java b/src/main/java/org/dataloader/DataLoaderRegistry.java index 9df6fc2..f75ddc4 100644 --- a/src/main/java/org/dataloader/DataLoaderRegistry.java +++ b/src/main/java/org/dataloader/DataLoaderRegistry.java @@ -1,7 +1,6 @@ package org.dataloader; import org.dataloader.stats.Statistics; -import org.dataloader.stats.StatisticsImpl; import java.util.ArrayList; import java.util.HashSet; @@ -100,7 +99,7 @@ public void dispatchAll() { * as the sum of all their statistics */ public Statistics getStatistics() { - Statistics stats = new StatisticsImpl(); + Statistics stats = new Statistics(); for (DataLoader dataLoader : dataLoaders.values()) { stats = stats.combine(dataLoader.getStatistics()); } diff --git a/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java index bd18bea..3c3624f 100644 --- a/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/NoOpStatisticsCollector.java @@ -5,7 +5,7 @@ */ public class NoOpStatisticsCollector implements StatisticsCollector { - private static final StatisticsImpl ZERO_STATS = new StatisticsImpl(); + private static final Statistics ZERO_STATS = new Statistics(); @Override public long incrementLoadCount() { diff --git a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java index 7e5b89d..af48b0c 100644 --- a/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java +++ b/src/main/java/org/dataloader/stats/SimpleStatisticsCollector.java @@ -45,7 +45,7 @@ public long incrementBatchLoadExceptionCount() { @Override public Statistics getStatistics() { - return new StatisticsImpl(loadCount.get(), loadErrorCount.get(), batchInvokeCount.get(), batchLoadCount.get(), batchLoadExceptionCount.get(), cacheHitCount.get()); + return new Statistics(loadCount.get(), loadErrorCount.get(), batchInvokeCount.get(), batchLoadCount.get(), batchLoadExceptionCount.get(), cacheHitCount.get()); } @Override diff --git a/src/main/java/org/dataloader/stats/Statistics.java b/src/main/java/org/dataloader/stats/Statistics.java index 58a4acc..bf8da1a 100644 --- a/src/main/java/org/dataloader/stats/Statistics.java +++ b/src/main/java/org/dataloader/stats/Statistics.java @@ -1,79 +1,172 @@ package org.dataloader.stats; +import java.util.LinkedHashMap; import java.util.Map; /** - * A simple set of statistics about {@link org.dataloader.DataLoader} operations + * This holds statistics on how a {@link org.dataloader.DataLoader} has performed */ -public interface Statistics { +public class Statistics { + + private final long loadCount; + private final long loadErrorCount; + private final long batchInvokeCount; + private final long batchLoadCount; + private final long batchLoadExceptionCount; + private final long cacheHitCount; + /** - * @return the number of objects {@link org.dataloader.DataLoader#load(Object)} has been asked to load + * Zero statistics */ - long getLoadCount(); + public Statistics() { + this(0, 0, 0, 0, 0, 0); + } + + public Statistics(long loadCount, long loadErrorCount, long batchInvokeCount, long batchLoadCount, long batchLoadExceptionCount, long cacheHitCount) { + this.loadCount = loadCount; + this.batchInvokeCount = batchInvokeCount; + this.batchLoadCount = batchLoadCount; + this.cacheHitCount = cacheHitCount; + this.batchLoadExceptionCount = batchLoadExceptionCount; + this.loadErrorCount = loadErrorCount; + } /** - * @return the number of times the {@link org.dataloader.DataLoader} batch loader function has been called + * A helper to divide two numbers and handle zero + * + * @param numerator the top bit + * @param denominator the bottom bit + * + * @return numerator / denominator returning zero when denominator is zero */ - long getBatchInvokeCount(); + public double ratio(long numerator, long denominator) { + return denominator == 0 ? 0f : ((double) numerator) / ((double) denominator); + } /** - * @return the number of objects that the {@link org.dataloader.DataLoader} batch loader function has been asked to load + * @return the number of objects {@link org.dataloader.DataLoader#load(Object)} has been asked to load */ - long getBatchLoadCount(); + public long getLoadCount() { + return loadCount; + } /** - * @return batchLoadCount / loadCount + * @return the number of times the {@link org.dataloader.DataLoader} batch loader function return an specific object that was in error */ - double getBatchLoadRatio(); + public long getLoadErrorCount() { + return loadErrorCount; + } /** - * @return the number of times {@link org.dataloader.DataLoader#load(Object)} resulted in a cache hit + * @return loadErrorCount / loadCount */ - long getCacheHitCount(); + public double getLoadErrorRatio() { + return ratio(loadErrorCount, loadCount); + } /** - * @return then number of times we missed the cache during {@link org.dataloader.DataLoader#load(Object)} + * @return the number of times the {@link org.dataloader.DataLoader} batch loader function has been called */ - long getCacheMissCount(); + public long getBatchInvokeCount() { + return batchInvokeCount; + } /** - * @return cacheHits / loadCount + * @return the number of objects that the {@link org.dataloader.DataLoader} batch loader function has been asked to load */ - double getCacheHitRatio(); + public long getBatchLoadCount() { + return batchLoadCount; + } + + /** + * @return batchLoadCount / loadCount + */ + public double getBatchLoadRatio() { + return ratio(batchLoadCount, loadCount); + } /** * @return the number of times the {@link org.dataloader.DataLoader} batch loader function throw an exception when trying to get any values */ - long getBatchLoadExceptionCount(); + public long getBatchLoadExceptionCount() { + return batchLoadExceptionCount; + } /** * @return batchLoadExceptionCount / loadCount */ - double getBatchLoadExceptionRatio(); + public double getBatchLoadExceptionRatio() { + return ratio(batchLoadExceptionCount, loadCount); + } /** - * @return the number of times the {@link org.dataloader.DataLoader} batch loader function return an specific object that was in error + * @return the number of times {@link org.dataloader.DataLoader#load(Object)} resulted in a cache hit */ - long getLoadErrorCount(); + public long getCacheHitCount() { + return cacheHitCount; + } + /** + * @return then number of times we missed the cache during {@link org.dataloader.DataLoader#load(Object)} + */ + public long getCacheMissCount() { + return loadCount - cacheHitCount; + } /** - * @return loadErrorCount / loadCount + * @return cacheHits / loadCount */ - double getLoadErrorRatio(); + public double getCacheHitRatio() { + return ratio(cacheHitCount, loadCount); + } /** * This will combine this set of statistics with another set of statistics so that they become the combined count of each * - * @param statistics the other statistics to combine + * @param other the other statistics to combine * * @return a new statistics object of the combined counts */ - Statistics combine(Statistics statistics); + public Statistics combine(Statistics other) { + return new Statistics( + this.loadCount + other.getLoadCount(), + this.loadErrorCount + other.getLoadErrorCount(), + this.batchInvokeCount + other.getBatchInvokeCount(), + this.batchLoadCount + other.getBatchLoadCount(), + this.batchLoadExceptionCount + other.getBatchLoadExceptionCount(), + this.cacheHitCount + other.getCacheHitCount() + ); + } /** * @return a map representation of the statistics, perhaps to send over JSON or some such */ - Map toMap(); + public Map toMap() { + Map stats = new LinkedHashMap<>(); + stats.put("loadCount", getLoadCount()); + stats.put("loadErrorCount", getLoadErrorCount()); + stats.put("loadErrorRatio", getLoadErrorRatio()); + + stats.put("batchInvokeCount", getBatchInvokeCount()); + stats.put("batchLoadCount", getBatchLoadCount()); + stats.put("batchLoadRatio", getBatchLoadRatio()); + stats.put("batchLoadExceptionCount", getBatchLoadExceptionCount()); + stats.put("batchLoadExceptionRatio", getBatchLoadExceptionRatio()); + + stats.put("cacheHitCount", getCacheHitCount()); + stats.put("cacheHitRatio", getCacheHitRatio()); + return stats; + } + + @Override + public String toString() { + return "Statistics{" + + "loadCount=" + loadCount + + ", loadErrorCount=" + loadErrorCount + + ", batchLoadCount=" + batchLoadCount + + ", batchLoadExceptionCount=" + batchLoadExceptionCount + + ", cacheHitCount=" + cacheHitCount + + '}'; + } } diff --git a/src/main/java/org/dataloader/stats/StatisticsImpl.java b/src/main/java/org/dataloader/stats/StatisticsImpl.java deleted file mode 100644 index b6442b5..0000000 --- a/src/main/java/org/dataloader/stats/StatisticsImpl.java +++ /dev/null @@ -1,132 +0,0 @@ -package org.dataloader.stats; - -import java.util.LinkedHashMap; -import java.util.Map; - -public class StatisticsImpl implements Statistics { - - private final long loadCount; - private final long loadErrorCount; - private final long batchInvokeCount; - private final long batchLoadCount; - private final long batchLoadExceptionCount; - private final long cacheHitCount; - - /** - * Zero statistics - */ - public StatisticsImpl() { - this(0, 0, 0, 0, 0, 0); - } - - public StatisticsImpl(long loadCount, long loadErrorCount, long batchInvokeCount, long batchLoadCount, long batchLoadExceptionCount, long cacheHitCount) { - this.loadCount = loadCount; - this.batchInvokeCount = batchInvokeCount; - this.batchLoadCount = batchLoadCount; - this.cacheHitCount = cacheHitCount; - this.batchLoadExceptionCount = batchLoadExceptionCount; - this.loadErrorCount = loadErrorCount; - } - - private double ratio(long numerator, long denominator) { - return denominator == 0 ? 0f : ((double) numerator) / ((double) denominator); - } - - @Override - public long getLoadCount() { - return loadCount; - } - - - @Override - public long getLoadErrorCount() { - return loadErrorCount; - } - - @Override - public double getLoadErrorRatio() { - return ratio(loadErrorCount, loadCount); - } - - @Override - public long getBatchInvokeCount() { - return batchInvokeCount; - } - - @Override - public long getBatchLoadCount() { - return batchLoadCount; - } - - @Override - public double getBatchLoadRatio() { - return ratio(batchLoadCount, loadCount); - } - - @Override - public long getBatchLoadExceptionCount() { - return batchLoadExceptionCount; - } - - @Override - public double getBatchLoadExceptionRatio() { - return ratio(batchLoadExceptionCount, loadCount); - } - - @Override - public long getCacheHitCount() { - return cacheHitCount; - } - - @Override - public long getCacheMissCount() { - return loadCount - cacheHitCount; - } - - @Override - public double getCacheHitRatio() { - return ratio(cacheHitCount, loadCount); - } - - - @Override - public Statistics combine(Statistics other) { - return new StatisticsImpl( - this.loadCount + other.getLoadCount(), - this.loadErrorCount + other.getLoadErrorCount(), - this.batchInvokeCount + other.getBatchInvokeCount(), - this.batchLoadCount + other.getBatchLoadCount(), - this.batchLoadExceptionCount + other.getBatchLoadExceptionCount(), - this.cacheHitCount + other.getCacheHitCount() - ); - } - - @Override - public Map toMap() { - Map stats = new LinkedHashMap<>(); - stats.put("loadCount", getLoadCount()); - stats.put("loadErrorCount", getLoadErrorCount()); - stats.put("loadErrorRatio", getLoadErrorRatio()); - - stats.put("batchInvokeCount", getBatchInvokeCount()); - stats.put("batchLoadCount", getBatchLoadCount()); - stats.put("batchLoadRatio", getBatchLoadRatio()); - stats.put("batchLoadExceptionCount", getBatchLoadExceptionCount()); - stats.put("batchLoadExceptionRatio", getBatchLoadExceptionRatio()); - - stats.put("cacheHitCount", getCacheHitCount()); - stats.put("cacheHitRatio", getCacheHitRatio()); - return stats; - } - - @Override - public String toString() { - return "StatisticsImpl{" + - "loadCount=" + loadCount + - ", loadErrorCount=" + loadErrorCount + - ", batchLoadCount=" + batchLoadCount + - ", batchLoadExceptionCount=" + batchLoadExceptionCount + - ", cacheHitCount=" + cacheHitCount + - '}'; - } -} diff --git a/src/test/java/org/dataloader/stats/StatisticsImplTest.java b/src/test/java/org/dataloader/stats/StatisticsTest.java similarity index 85% rename from src/test/java/org/dataloader/stats/StatisticsImplTest.java rename to src/test/java/org/dataloader/stats/StatisticsTest.java index 87adebe..b900807 100644 --- a/src/test/java/org/dataloader/stats/StatisticsImplTest.java +++ b/src/test/java/org/dataloader/stats/StatisticsTest.java @@ -7,12 +7,12 @@ import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; -public class StatisticsImplTest { +public class StatisticsTest { @Test public void combine_works() throws Exception { - StatisticsImpl one = new StatisticsImpl(1, 5, 6, 2, 4, 3); - StatisticsImpl two = new StatisticsImpl(6, 10, 6, 7, 9, 8); + Statistics one = new Statistics(1, 5, 6, 2, 4, 3); + Statistics two = new Statistics(6, 10, 6, 7, 9, 8); Statistics combine = one.combine(two); @@ -27,7 +27,7 @@ public void combine_works() throws Exception { @Test public void to_map_works() throws Exception { - StatisticsImpl one = new StatisticsImpl(10, 2, 11, 3, 4, 5); + Statistics one = new Statistics(10, 2, 11, 3, 4, 5); Map map = one.toMap(); assertThat(map.get("loadCount"), equalTo(10L));