From 6daeb1a04c92e467e6b305fbe16539cca5afa959 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 7 Jul 2020 09:40:39 +0200 Subject: [PATCH] DATAREDIS-1082 - Expose cache metrics for RedisCache. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now expose CacheStatistics for RedisCache. Each cache tracks its statistics (retrievals, hits, misses, stores, removals, lock wait) locally. CacheStatistics is exposed through RedisCacheWriter to allow for statistics customization depending on the cache writer strategy. The statistic object is a singleton per cache instance that gets updated with as the cache operates. RedisCache cache = …; CacheStatistics statistics = cache.getStatistics(); statistics.getRetrievals(); statistics.getHits(); Original Pull Request: #545 --- src/main/asciidoc/new-features.adoc | 6 + .../data/redis/cache/CacheStatistics.java | 58 ++++++++ .../redis/cache/DefaultCacheStatistics.java | 125 ++++++++++++++++++ .../redis/cache/DefaultRedisCacheWriter.java | 32 ++++- .../data/redis/cache/RedisCache.java | 11 ++ .../data/redis/cache/RedisCacheWriter.java | 9 ++ .../DefaultCacheStatisticsUnitTests.java | 80 +++++++++++ .../cache/DefaultRedisCacheWriterTests.java | 65 ++++++--- 8 files changed, 363 insertions(+), 23 deletions(-) create mode 100644 src/main/java/org/springframework/data/redis/cache/CacheStatistics.java create mode 100644 src/main/java/org/springframework/data/redis/cache/DefaultCacheStatistics.java create mode 100644 src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsUnitTests.java diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc index ee23268a4..5ae24df58 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -3,7 +3,13 @@ This section briefly covers items that are new and noteworthy in the latest releases. +[[new-in-2.4.0]] +== New in Spring Data Redis 2.4 + +* `RedisCache` now exposes `CacheStatistics`. + [[new-in-2.3.0]] + == New in Spring Data Redis 2.3 * Template API Method Refinements for `Duration` and `Instant`. diff --git a/src/main/java/org/springframework/data/redis/cache/CacheStatistics.java b/src/main/java/org/springframework/data/redis/cache/CacheStatistics.java new file mode 100644 index 000000000..3edd24553 --- /dev/null +++ b/src/main/java/org/springframework/data/redis/cache/CacheStatistics.java @@ -0,0 +1,58 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.cache; + +import java.util.concurrent.TimeUnit; + +/** + * Cache statistics for a Redis Cache. + * + * @author Mark Paluch + * @since 2.4 + */ +public interface CacheStatistics { + + /** + * @return number of put operations on the cache. + */ + long getStores(); + + /** + * @return number of get operations. + */ + long getRetrievals(); + + /** + * @return number of cache get hits. + */ + long getHits(); + + /** + * @return number of cache get misses. + */ + long getMisses(); + + /** + * @return number of cache removals. + */ + long getRemovals(); + + /** + * @param unit the time unit to report the lock wait duration. + * @return lock duration using the given {@link TimeUnit} if the cache is configured to use locking. + */ + long getLockWaitDuration(TimeUnit unit); +} diff --git a/src/main/java/org/springframework/data/redis/cache/DefaultCacheStatistics.java b/src/main/java/org/springframework/data/redis/cache/DefaultCacheStatistics.java new file mode 100644 index 000000000..e2b9d90e2 --- /dev/null +++ b/src/main/java/org/springframework/data/redis/cache/DefaultCacheStatistics.java @@ -0,0 +1,125 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.cache; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.LongAdder; + +import org.springframework.util.Assert; + +/** + * Default mutable {@link CacheStatistics} implementation. + * + * @author Mark Paluch + * @since 2.4 + */ +class DefaultCacheStatistics implements CacheStatistics { + + private final LongAdder stores = new LongAdder(); + private final LongAdder retrievals = new LongAdder(); + private final LongAdder hits = new LongAdder(); + private final LongAdder misses = new LongAdder(); + private final LongAdder removals = new LongAdder(); + private final LongAdder lockWaitTimeNs = new LongAdder(); + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getStores() + */ + @Override + public long getStores() { + return stores.sum(); + } + + void incrementStores() { + stores.increment(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getRetrievals() + */ + @Override + public long getRetrievals() { + return retrievals.sum(); + } + + void incrementRetrievals() { + retrievals.increment(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getHits() + */ + @Override + public long getHits() { + return hits.sum(); + } + + void incrementHits() { + hits.increment(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getMisses() + */ + @Override + public long getMisses() { + return misses.sum(); + } + + void incrementMisses() { + misses.increment(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getRemovals() + */ + @Override + public long getRemovals() { + return removals.sum(); + } + + void incrementRemovals() { + incrementRemovals(1); + } + + /** + * @param x number of removals to add. + */ + void incrementRemovals(int x) { + removals.add(x); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.CacheStatistics#getLockWaitDuration(java.util.concurrent.TimeUnit) + */ + @Override + public long getLockWaitDuration(TimeUnit unit) { + + Assert.notNull(unit, "TimeUnit must not be null"); + + return unit.convert(lockWaitTimeNs.sum(), TimeUnit.NANOSECONDS); + } + + void incrementLockWaitTime(long waitTimeNs) { + lockWaitTimeNs.add(waitTimeNs); + } +} 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 698f14778..ca9f2ff65 100644 --- a/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java +++ b/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java @@ -51,6 +51,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { private final RedisConnectionFactory connectionFactory; private final Duration sleepTime; + private final DefaultCacheStatistics statistics; /** * @param connectionFactory must not be {@literal null}. @@ -71,6 +72,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { this.connectionFactory = connectionFactory; this.sleepTime = sleepTime; + this.statistics = new DefaultCacheStatistics(); } /* @@ -94,6 +96,8 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { return "OK"; }); + + statistics.incrementStores(); } /* @@ -106,7 +110,17 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { Assert.notNull(name, "Name must not be null!"); Assert.notNull(key, "Key must not be null!"); - return execute(name, connection -> connection.get(key)); + byte[] result = execute(name, connection -> connection.get(key)); + + statistics.incrementRetrievals(); + + if (result != null) { + statistics.incrementHits(); + } else { + statistics.incrementMisses(); + } + + return result; } /* @@ -132,6 +146,8 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { if (shouldExpireWithin(ttl)) { connection.pExpire(key, ttl.toMillis()); } + + statistics.incrementStores(); return null; } @@ -156,6 +172,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { Assert.notNull(key, "Key must not be null!"); execute(name, connection -> connection.del(key)); + statistics.incrementRemovals(); } /* @@ -183,6 +200,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { .toArray(new byte[0][]); if (keys.length > 0) { + statistics.incrementRemovals(keys.length); connection.del(keys); } } finally { @@ -196,6 +214,15 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { }); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.RedisCacheWriter#getStatistics() + */ + @Override + public DefaultCacheStatistics getStatistics() { + return statistics; + } + /** * Explicitly set a write lock on a cache. * @@ -262,6 +289,7 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { return; } + long lockWaitTimeNs = System.nanoTime(); try { while (doCheckLock(name, connection)) { @@ -274,6 +302,8 @@ class DefaultRedisCacheWriter implements RedisCacheWriter { throw new PessimisticLockingFailureException(String.format("Interrupted while waiting to unlock cache %s", name), ex); + } finally { + statistics.incrementLockWaitTime(System.nanoTime() - lockWaitTimeNs); } } 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 e9ac870ac..7dce2e81c 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -193,6 +193,17 @@ public class RedisCache extends AbstractValueAdaptingCache { cacheWriter.clean(name, pattern); } + /** + * Return the {@link CacheStatistics} for this cache instance. Statistics are accumulated per cache instance and not + * from the backing Redis data store. Cache statistics are accumulated starting from the time a cache is created. + * + * @return statistics object for this {@link RedisCache}. + * @since 2.4 + */ + public CacheStatistics getStatistics() { + return cacheWriter.getStatistics(); + } + /** * Get {@link RedisCacheConfiguration} used. * 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 33ace1221..9a3ba701f 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java @@ -106,4 +106,13 @@ public interface RedisCacheWriter { * @param pattern The pattern for the keys to remove. Must not be {@literal null}. */ void clean(String name, byte[] pattern); + + /** + * Return the {@link CacheStatistics} for this cache instance. Statistics are accumulated per cache instance and not + * from the backing Redis data store. Cache statistics are accumulated starting from the time a cache is created. + * + * @return statistics object for this {@link RedisCache}. + * @since 2.4 + */ + CacheStatistics getStatistics(); } diff --git a/src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsUnitTests.java b/src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsUnitTests.java new file mode 100644 index 000000000..a1436deaf --- /dev/null +++ b/src/test/java/org/springframework/data/redis/cache/DefaultCacheStatisticsUnitTests.java @@ -0,0 +1,80 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.cache; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link DefaultCacheStatistics}. + * + * @author Mark Paluch + */ +class DefaultCacheStatisticsUnitTests { + + DefaultCacheStatistics statistics = new DefaultCacheStatistics(); + + @Test // DATAREDIS-1082 + void shouldReportRetrievals() { + + assertThat(statistics.getRetrievals()).isZero(); + + statistics.incrementRetrievals(); + + assertThat(statistics.getRetrievals()).isOne(); + } + + @Test // DATAREDIS-1082 + void shouldReportHits() { + + assertThat(statistics.getHits()).isZero(); + + statistics.incrementHits(); + + assertThat(statistics.getHits()).isOne(); + } + + @Test // DATAREDIS-1082 + void shouldReportMisses() { + + assertThat(statistics.getMisses()).isZero(); + + statistics.incrementMisses(); + + assertThat(statistics.getMisses()).isOne(); + } + + @Test // DATAREDIS-1082 + void shouldReportPuts() { + + assertThat(statistics.getStores()).isZero(); + + statistics.incrementStores(); + + assertThat(statistics.getStores()).isOne(); + } + + @Test // DATAREDIS-1082 + void shouldReportRemovals() { + + assertThat(statistics.getRemovals()).isZero(); + + statistics.incrementRemovals(); + + assertThat(statistics.getRemovals()).isOne(); + } +} 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 2faf4a547..741cd705c 100644 --- a/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java +++ b/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java @@ -41,6 +41,8 @@ import org.springframework.data.redis.connection.jedis.JedisConnectionFactory; import org.springframework.data.redis.core.types.Expiration; /** + * Integration tests for {@link DefaultRedisCacheWriter}. + * * @author Christoph Strobl * @author Mark Paluch */ @@ -85,15 +87,19 @@ public class DefaultRedisCacheWriterTests { doWithConnection(RedisConnection::flushAll); } - @Test // DATAREDIS-481 + @Test // DATAREDIS-481, DATAREDIS-1082 public void putShouldAddEternalEntry() { - nonLockingRedisCacheWriter(connectionFactory).put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory); + writer.put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); doWithConnection(connection -> { assertThat(connection.get(binaryCacheKey)).isEqualTo(binaryCacheValue); assertThat(connection.ttl(binaryCacheKey)).isEqualTo(-1); }); + + assertThat(writer.getStatistics().getStores()).isOne(); + assertThat(writer.getStatistics().getLockWaitDuration(TimeUnit.NANOSECONDS)).isZero(); } @Test // DATAREDIS-481 @@ -136,13 +142,18 @@ public class DefaultRedisCacheWriterTests { }); } - @Test // DATAREDIS-481 + @Test // DATAREDIS-481, DATAREDIS-1082 public void getShouldReturnValue() { doWithConnection(connection -> connection.set(binaryCacheKey, binaryCacheValue)); - assertThat(nonLockingRedisCacheWriter(connectionFactory).get(CACHE_NAME, binaryCacheKey)) + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory); + assertThat(writer.get(CACHE_NAME, binaryCacheKey)) .isEqualTo(binaryCacheValue); + + assertThat(writer.getStatistics().getRetrievals()).isOne(); + assertThat(writer.getStatistics().getHits()).isOne(); + assertThat(writer.getStatistics().getMisses()).isZero(); } @Test // DATAREDIS-481 @@ -150,52 +161,61 @@ public class DefaultRedisCacheWriterTests { assertThat(nonLockingRedisCacheWriter(connectionFactory).get(CACHE_NAME, binaryCacheKey)).isNull(); } - @Test // DATAREDIS-481 + @Test // DATAREDIS-481, DATAREDIS-1082 public void putIfAbsentShouldAddEternalEntryWhenKeyDoesNotExist() { - assertThat(nonLockingRedisCacheWriter(connectionFactory).putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory); + assertThat(writer.putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO)).isNull(); doWithConnection(connection -> { assertThat(connection.get(binaryCacheKey)).isEqualTo(binaryCacheValue); }); + + assertThat(writer.getStatistics().getStores()).isOne(); } - @Test // DATAREDIS-481 + @Test // DATAREDIS-481, DATAREDIS-1082 public void putIfAbsentShouldNotAddEternalEntryWhenKeyAlreadyExist() { doWithConnection(connection -> connection.set(binaryCacheKey, binaryCacheValue)); - assertThat(nonLockingRedisCacheWriter(connectionFactory).putIfAbsent(CACHE_NAME, binaryCacheKey, "foo".getBytes(), - Duration.ZERO)).isEqualTo(binaryCacheValue); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory); + assertThat(writer.putIfAbsent(CACHE_NAME, binaryCacheKey, "foo".getBytes(), Duration.ZERO)) + .isEqualTo(binaryCacheValue); doWithConnection(connection -> { assertThat(connection.get(binaryCacheKey)).isEqualTo(binaryCacheValue); }); + + assertThat(writer.getStatistics().getStores()).isZero(); } - @Test // DATAREDIS-481 + @Test // DATAREDIS-481, DATAREDIS-1082 public void putIfAbsentShouldAddExpiringEntryWhenKeyDoesNotExist() { - assertThat(nonLockingRedisCacheWriter(connectionFactory).putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, - Duration.ofSeconds(5))).isNull(); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory); + assertThat(writer.putIfAbsent(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ofSeconds(5))).isNull(); doWithConnection(connection -> { assertThat(connection.ttl(binaryCacheKey)).isGreaterThan(3).isLessThan(6); }); + assertThat(writer.getStatistics().getStores()).isOne(); } - @Test // DATAREDIS-481 + @Test // DATAREDIS-481, DATAREDIS-1082 public void removeShouldDeleteEntry() { doWithConnection(connection -> connection.set(binaryCacheKey, binaryCacheValue)); - nonLockingRedisCacheWriter(connectionFactory).remove(CACHE_NAME, binaryCacheKey); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory); + writer.remove(CACHE_NAME, binaryCacheKey); doWithConnection(connection -> assertThat(connection.exists(binaryCacheKey)).isFalse()); + assertThat(writer.getStatistics().getRemovals()).isOne(); } - @Test // DATAREDIS-418 + @Test // DATAREDIS-418, DATAREDIS-1082 public void cleanShouldRemoveAllKeysByPattern() { doWithConnection(connection -> { @@ -203,13 +223,14 @@ public class DefaultRedisCacheWriterTests { connection.set("foo".getBytes(), "bar".getBytes()); }); - nonLockingRedisCacheWriter(connectionFactory).clean(CACHE_NAME, - (CACHE_NAME + "::*").getBytes(Charset.forName("UTF-8"))); + RedisCacheWriter writer = nonLockingRedisCacheWriter(connectionFactory); + writer.clean(CACHE_NAME, (CACHE_NAME + "::*").getBytes(Charset.forName("UTF-8"))); doWithConnection(connection -> { assertThat(connection.exists(binaryCacheKey)).isFalse(); assertThat(connection.exists("foo".getBytes())).isTrue(); }); + assertThat(writer.getStatistics().getRemovals()).isOne(); } @Test // DATAREDIS-481 @@ -237,18 +258,17 @@ public class DefaultRedisCacheWriterTests { }); } - @Test // DATAREDIS-481 + @Test // DATAREDIS-481, DATAREDIS-1082 public void lockingCacheWriterShouldWaitForLockRelease() throws InterruptedException { - DefaultRedisCacheWriter cw = (DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory); - cw.lock(CACHE_NAME); + DefaultRedisCacheWriter writer = (DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory); + writer.lock(CACHE_NAME); CountDownLatch beforeWrite = new CountDownLatch(1); CountDownLatch afterWrite = new CountDownLatch(1); Thread th = new Thread(() -> { - RedisCacheWriter writer = lockingRedisCacheWriter(connectionFactory); beforeWrite.countDown(); writer.put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); afterWrite.countDown(); @@ -265,12 +285,13 @@ public class DefaultRedisCacheWriterTests { assertThat(connection.exists(binaryCacheKey)).isFalse(); }); - cw.unlock(CACHE_NAME); + writer.unlock(CACHE_NAME); afterWrite.await(); doWithConnection(connection -> { assertThat(connection.exists(binaryCacheKey)).isTrue(); }); + assertThat(writer.getStatistics().getLockWaitDuration(TimeUnit.NANOSECONDS)).isGreaterThan(0); } finally { th.interrupt(); }