diff --git a/src/main/asciidoc/reference/redis.adoc b/src/main/asciidoc/reference/redis.adoc index 1383e5101..2ea34a792 100644 --- a/src/main/asciidoc/reference/redis.adoc +++ b/src/main/asciidoc/reference/redis.adoc @@ -812,5 +812,6 @@ The following table lists the default settings for `RedisCacheConfiguration`: [NOTE] ==== -By default `RedisCache` does not expose statistics. Use `RedisCacheManagerBuilder.enableStatistics()` to collect local _hits_ and _misses_ exposed via `RedisCache#getStatistics()`, that returns a snapshot of the collected data. +By default `RedisCache`, statistics are disabled. +Use `RedisCacheManagerBuilder.enableStatistics()` to collect local _hits_ and _misses_ through `RedisCache#getStatistics()`, returning a snapshot of the collected data. ==== diff --git a/src/main/java/org/springframework/data/redis/cache/CacheStatistics.java b/src/main/java/org/springframework/data/redis/cache/CacheStatistics.java index b831238b8..ef0c4763f 100644 --- a/src/main/java/org/springframework/data/redis/cache/CacheStatistics.java +++ b/src/main/java/org/springframework/data/redis/cache/CacheStatistics.java @@ -79,7 +79,7 @@ public interface CacheStatistics { Instant getSince(); /** - * @return instantaneous point in time of last statistics counter reset. Equals {@link #getSince()} if never resetted. + * @return instantaneous point in time of last statistics counter reset. Equals {@link #getSince()} if never reset. */ Instant getLastReset(); diff --git a/src/main/java/org/springframework/data/redis/cache/CacheStatisticsCollector.java b/src/main/java/org/springframework/data/redis/cache/CacheStatisticsCollector.java index 6d18cbd83..95097e7e7 100644 --- a/src/main/java/org/springframework/data/redis/cache/CacheStatisticsCollector.java +++ b/src/main/java/org/springframework/data/redis/cache/CacheStatisticsCollector.java @@ -26,35 +26,35 @@ public interface CacheStatisticsCollector extends CacheStatisticsProvider { /** * Increase the counter for {@literal put operations} of the given cache. - * + * * @param cacheName must not be {@literal null}. */ void incPuts(String cacheName); /** * Increase the counter for {@literal get operations} of the given cache. - * + * * @param cacheName must not be {@literal null}. */ void incGets(String cacheName); /** * Increase the counter for {@literal get operations with result} of the given cache. - * + * * @param cacheName must not be {@literal null}. */ void incHits(String cacheName); /** * Increase the counter for {@literal get operations without result} of the given cache. - * + * * @param cacheName must not be {@literal null}. */ void incMisses(String cacheName); /** * Increase the counter for {@literal delete operations} of the given cache. - * + * * @param cacheName must not be {@literal null}. */ default void incDeletes(String cacheName) { @@ -63,21 +63,21 @@ public interface CacheStatisticsCollector extends CacheStatisticsProvider { /** * Increase the counter for {@literal delete operations} of the given cache by the given value. - * + * * @param cacheName must not be {@literal null}. */ void incDeletesBy(String cacheName, int value); /** * Increase the gauge for {@literal sync lock duration} of the cache by the given nanoseconds. - * + * * @param cacheName must not be {@literal null}. */ void incLockTime(String cacheName, long durationNS); /** * Reset the all counters and gauges of for the given cache. - * + * * @param cacheName must not be {@literal null}. */ void reset(String cacheName); @@ -92,7 +92,7 @@ public interface CacheStatisticsCollector extends CacheStatisticsProvider { /** * @return a default {@link CacheStatisticsCollector} implementation. */ - static CacheStatisticsCollector instance/*clearlyNeedsBetterNaming*/() { + static CacheStatisticsCollector create() { return new DefaultCacheStatisticsCollector(); } } diff --git a/src/main/java/org/springframework/data/redis/cache/CacheStatisticsProvider.java b/src/main/java/org/springframework/data/redis/cache/CacheStatisticsProvider.java index bd59f454a..6b675df6b 100644 --- a/src/main/java/org/springframework/data/redis/cache/CacheStatisticsProvider.java +++ b/src/main/java/org/springframework/data/redis/cache/CacheStatisticsProvider.java @@ -16,13 +16,18 @@ package org.springframework.data.redis.cache; /** + * Interface to be implemented by objects that expose {@link CacheStatistics} identified by {@code cacheName}. Typically + * used by cache writers. + * * @author Christoph Strobl + * @author Mark Paluch * @since 2.4 */ public interface CacheStatisticsProvider { /** - * Obtain snapshot of the captured statistics. + * Obtain snapshot of the captured statistics. May return a statistics object whose counters are zero if there are no + * statistics for {@code cacheName}. * * @param cacheName must not be {@literal null}. * @return never {@literal null}. diff --git a/src/main/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollector.java b/src/main/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollector.java index 4788d71cc..c2f86b661 100644 --- a/src/main/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollector.java +++ b/src/main/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollector.java @@ -21,7 +21,7 @@ import java.util.concurrent.ConcurrentHashMap; /** * Default {@link CacheStatisticsCollector} implementation holding synchronized per cache * {@link MutableCacheStatistics}. - * + * * @author Christoph Strobl * @since 2.4 */ diff --git a/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java b/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java index ec0f4fcc1..3839f336b 100644 --- a/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java +++ b/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java @@ -244,8 +244,12 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { statistics.reset(name); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.RedisCacheWriter#with(CacheStatisticsCollector) + */ @Override - public RedisCacheWriter with(CacheStatisticsCollector cacheStatisticsCollector) { + public RedisCacheWriter withStatisticsCollector(CacheStatisticsCollector cacheStatisticsCollector) { return new DefaultRedisCacheWriter(connectionFactory, sleepTime, cacheStatisticsCollector); } diff --git a/src/main/java/org/springframework/data/redis/cache/MutableCacheStatistics.java b/src/main/java/org/springframework/data/redis/cache/MutableCacheStatistics.java index 09db99d47..3db1ce81e 100644 --- a/src/main/java/org/springframework/data/redis/cache/MutableCacheStatistics.java +++ b/src/main/java/org/springframework/data/redis/cache/MutableCacheStatistics.java @@ -115,10 +115,6 @@ class MutableCacheStatistics implements CacheStatistics { return deletes.sum(); } - void incDeletes() { - incDeletes(1); - } - /** * @param x number of removals to add. */ @@ -138,11 +134,19 @@ class MutableCacheStatistics implements CacheStatistics { return unit.convert(lockWaitTimeNs.sum(), TimeUnit.NANOSECONDS); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getSince() + */ @Override public Instant getSince() { return this.aliveSince; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getLastReset() + */ @Override public Instant getLastReset() { return lastReset; @@ -271,6 +275,9 @@ class MutableCacheStatistics implements CacheStatistics { */ @Override public long getLockWaitDuration(TimeUnit unit) { + + Assert.notNull(unit, "TimeUnit must not be null"); + return unit.convert(lockWaitTimeNS, TimeUnit.NANOSECONDS); } diff --git a/src/main/java/org/springframework/data/redis/cache/NoOpCacheStatisticsCollector.java b/src/main/java/org/springframework/data/redis/cache/NoOpCacheStatisticsCollector.java index cd41d6deb..ac207532a 100644 --- a/src/main/java/org/springframework/data/redis/cache/NoOpCacheStatisticsCollector.java +++ b/src/main/java/org/springframework/data/redis/cache/NoOpCacheStatisticsCollector.java @@ -23,8 +23,9 @@ import org.springframework.util.ObjectUtils; /** * {@link CacheStatisticsCollector} implementation that does not capture anything but throws an * {@link IllegalStateException} when {@link #getCacheStatistics(String) obtaining} {@link CacheStatistics} for a cache. - * + * * @author Christoph Strobl + * @author Mark Paluch * @since 2.4 */ enum NoOpCacheStatisticsCollector implements CacheStatisticsCollector { @@ -86,62 +87,102 @@ enum NoOpCacheStatisticsCollector implements CacheStatisticsCollector { */ @Override public CacheStatistics getCacheStatistics(String cacheName) { - return new EmptyStats(cacheName); + return new EmptyStatistics(cacheName); } - private static class EmptyStats implements CacheStatistics { + private static class EmptyStatistics implements CacheStatistics { private final String cacheName; - EmptyStats(String cacheName) { + EmptyStatistics(String cacheName) { this.cacheName = cacheName; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getCacheName() + */ @Override public String getCacheName() { return cacheName; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getPuts() + */ @Override public long getPuts() { - return -1; + return 0; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getGets() + */ @Override public long getGets() { - return -1; + return 0; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getHits() + */ @Override public long getHits() { - return -1; + return 0; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getMisses() + */ @Override public long getMisses() { - return -1; + return 0; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getDeletes() + */ @Override public long getDeletes() { - return -1; + return 0; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getLockWaitDuration(java.util.concurrent.TimeUnit) + */ @Override public long getLockWaitDuration(TimeUnit unit) { - return -1; + return 0; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getSince() + */ @Override public Instant getSince() { return Instant.EPOCH; } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getLastReset() + */ @Override public Instant getLastReset() { return getSince(); } + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ @Override public boolean equals(Object o) { @@ -152,10 +193,14 @@ enum NoOpCacheStatisticsCollector implements CacheStatisticsCollector { return false; } - EmptyStats that = (EmptyStats) o; + EmptyStatistics that = (EmptyStatistics) o; return ObjectUtils.nullSafeEquals(cacheName, that.cacheName); } + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ @Override public int hashCode() { return ObjectUtils.nullSafeHashCode(cacheName); diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCache.java b/src/main/java/org/springframework/data/redis/cache/RedisCache.java index 88dea0de2..0bb2a6051 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -194,12 +194,11 @@ public class RedisCache extends AbstractValueAdaptingCache { } /** - * Return the {@link CacheStatistics} for this cache instance. Statistics are accumulated per cache instance and not - * from the backing Redis data store. + * Return the {@link CacheStatistics} snapshot for this cache instance. Statistics are accumulated per cache instance + * and not from the backing Redis data store. * * @return statistics object for this {@link RedisCache}. * @since 2.4 - * @throws IllegalStateException if the {@link RedisCacheWriter} does not support statistics. */ public CacheStatistics getStatistics() { return cacheWriter.getCacheStatistics(getName()); diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java b/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java index b15cbf269..a80f7f918 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java @@ -462,17 +462,8 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * @since 2.4 */ public RedisCacheManagerBuilder enableStatistics() { - return statisticsCollector(CacheStatisticsCollector.instance()); - } - /** - * @return - * @since 2.4 - */ - private RedisCacheManagerBuilder statisticsCollector(CacheStatisticsCollector statisticsCollector) { - - Assert.notNull(statisticsCollector, "StatisticsCollector must not be null!"); - this.statisticsCollector = statisticsCollector; + this.statisticsCollector = CacheStatisticsCollector.create(); return this; } @@ -489,7 +480,7 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager RedisCacheWriter theCacheWriter = cacheWriter; if (!statisticsCollector.equals(CacheStatisticsCollector.none())) { - theCacheWriter = cacheWriter.with(statisticsCollector); + theCacheWriter = cacheWriter.withStatisticsCollector(statisticsCollector); } RedisCacheManager cm = new RedisCacheManager(theCacheWriter, defaultCacheConfiguration, initialCaches, diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java b/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java index 9c2525790..b67ea9e1c 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java @@ -120,6 +120,6 @@ public interface RedisCacheWriter extends CacheStatisticsProvider { * @param cacheStatisticsCollector must not be {@literal null}. * @return new instance of {@link RedisCacheWriter}. */ - RedisCacheWriter with(CacheStatisticsCollector cacheStatisticsCollector); + RedisCacheWriter withStatisticsCollector(CacheStatisticsCollector cacheStatisticsCollector); } diff --git a/src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollectorUnitTests.java b/src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollectorUnitTests.java index 073f83ecc..806a01126 100644 --- a/src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollectorUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsCollectorUnitTests.java @@ -21,6 +21,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; /** + * Unit tests for {@link DefaultCacheStatisticsCollector}. + * * @author Christoph Strobl */ class DefaultCacheStatisticsCollectorUnitTests { diff --git a/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java b/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java index a2228a955..c8b8da833 100644 --- a/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java +++ b/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java @@ -90,7 +90,8 @@ public class DefaultRedisCacheWriterTests { @Test // DATAREDIS-481, DATAREDIS-1082 public void putShouldAddEternalEntry() { - RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); writer.put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); doWithConnection(connection -> { @@ -147,7 +148,8 @@ public class DefaultRedisCacheWriterTests { doWithConnection(connection -> connection.set(binaryCacheKey, binaryCacheValue)); - RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); assertThat(writer.get(CACHE_NAME, binaryCacheKey)) .isEqualTo(binaryCacheValue); @@ -164,7 +166,8 @@ public class DefaultRedisCacheWriterTests { @Test // DATAREDIS-481, DATAREDIS-1082 public void putIfAbsentShouldAddEternalEntryWhenKeyDoesNotExist() { - RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); assertThat(writer.putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO)).isNull(); @@ -180,7 +183,8 @@ public class DefaultRedisCacheWriterTests { doWithConnection(connection -> connection.set(binaryCacheKey, binaryCacheValue)); - RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); assertThat(writer.putIfAbsent(CACHE_NAME, binaryCacheKey, "foo".getBytes(), Duration.ZERO)) .isEqualTo(binaryCacheValue); @@ -194,7 +198,8 @@ public class DefaultRedisCacheWriterTests { @Test // DATAREDIS-481, DATAREDIS-1082 public void putIfAbsentShouldAddExpiringEntryWhenKeyDoesNotExist() { - RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); assertThat(writer.putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ofSeconds(5))).isNull(); doWithConnection(connection -> { @@ -208,7 +213,8 @@ public class DefaultRedisCacheWriterTests { doWithConnection(connection -> connection.set(binaryCacheKey, binaryCacheValue)); - RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); writer.remove(CACHE_NAME, binaryCacheKey); doWithConnection(connection -> assertThat(connection.exists(binaryCacheKey)).isFalse()); @@ -223,7 +229,8 @@ public class DefaultRedisCacheWriterTests { connection.set("foo".getBytes(), "bar".getBytes()); }); - RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); writer.clean(CACHE_NAME, (CACHE_NAME + "::*").getBytes(Charset.forName("UTF-8"))); doWithConnection(connection -> { @@ -261,7 +268,8 @@ public class DefaultRedisCacheWriterTests { @Test // DATAREDIS-481, DATAREDIS-1082 public void lockingCacheWriterShouldWaitForLockRelease() throws InterruptedException { - DefaultRedisCacheWriter writer = (DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory).with(CacheStatisticsCollector.instance()); + DefaultRedisCacheWriter writer = (DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory) + .withStatisticsCollector(CacheStatisticsCollector.create()); writer.lock(CACHE_NAME); CountDownLatch beforeWrite = new CountDownLatch(1); @@ -347,7 +355,7 @@ public class DefaultRedisCacheWriterTests { cw.putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ofSeconds(5)); assertThat(stats).isNotNull(); - assertThat(stats.getPuts()).isNegative(); + assertThat(stats.getPuts()).isZero(); } private void doWithConnection(Consumer callback) { diff --git a/src/test/java/org/springframework/data/redis/cache/MutableCacheStatisticsUnitTests.java b/src/test/java/org/springframework/data/redis/cache/MutableCacheStatisticsUnitTests.java index 1dc305a5a..1fe51fe5c 100644 --- a/src/test/java/org/springframework/data/redis/cache/MutableCacheStatisticsUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/MutableCacheStatisticsUnitTests.java @@ -73,7 +73,7 @@ class MutableCacheStatisticsUnitTests { assertThat(statistics.getDeletes()).isZero(); - statistics.incDeletes(); + statistics.incDeletes(1); assertThat(statistics.getDeletes()).isOne(); } diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java index aa5c37500..b70e43a56 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java @@ -175,10 +175,10 @@ class RedisCacheManagerUnitTests { @Test // DATAREDIS-1082 void builderSetsStatisticsCollectorWhenEnabled() { - when(cacheWriter.with(any())).thenReturn(cacheWriter); + when(cacheWriter.withStatisticsCollector(any())).thenReturn(cacheWriter); RedisCacheManager.builder(cacheWriter).enableStatistics().build(); - verify(cacheWriter).with(any(DefaultCacheStatisticsCollector.class)); + verify(cacheWriter).withStatisticsCollector(any(DefaultCacheStatisticsCollector.class)); } @Test // DATAREDIS-1082 @@ -186,6 +186,6 @@ class RedisCacheManagerUnitTests { RedisCacheManager.builder(cacheWriter).build(); - verify(cacheWriter, never()).with(any()); + verify(cacheWriter, never()).withStatisticsCollector(any()); } }