You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by mc...@apache.org on 2020/08/19 14:54:06 UTC
[cassandra] branch trunk updated: Improved testing for CacheMetrics
and ChunkCacheMetrics Fixed failing assertions in CachingBench.
This is an automated email from the ASF dual-hosted git repository.
mck pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 0d99470 Improved testing for CacheMetrics and ChunkCacheMetrics Fixed failing assertions in CachingBench.
0d99470 is described below
commit 0d99470f154a86c74665fed5f33a65ef146c4c31
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Tue May 5 10:15:00 2020 -0400
Improved testing for CacheMetrics and ChunkCacheMetrics
Fixed failing assertions in CachingBench.
patch by Stephen Mallette; reviewed by Mick Semb Wever, Branimir Lambov for CASSANDRA-15788
---
CHANGES.txt | 1 +
.../org/apache/cassandra/cache/ChunkCache.java | 2 +-
.../apache/cassandra/cache/InstrumentingCache.java | 3 +
.../org/apache/cassandra/io/util/FileHandle.java | 2 +-
.../org/apache/cassandra/metrics/CacheMetrics.java | 9 +
.../cassandra/metrics/ChunkCacheMetrics.java | 9 -
.../org/apache/cassandra/cql3/CachingBench.java | 9 +-
.../apache/cassandra/metrics/CacheMetricsTest.java | 206 +++++++++++++++++++++
8 files changed, 229 insertions(+), 12 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index a3d66cf..d617dec 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.0-beta2
+ * Improved testability for CacheMetrics and ChunkCacheMetrics (CASSANDRA-15788)
* Handle errors in StreamSession#prepare (CASSANDRA-15852)
* FQL replay should have options to ignore DDL statements (CASSANDRA-16039)
* Remove COMPACT STORAGE internals (CASSANDRA-13994)
diff --git a/src/java/org/apache/cassandra/cache/ChunkCache.java b/src/java/org/apache/cassandra/cache/ChunkCache.java
index 0edb681..e370206 100644
--- a/src/java/org/apache/cassandra/cache/ChunkCache.java
+++ b/src/java/org/apache/cassandra/cache/ChunkCache.java
@@ -166,7 +166,7 @@ public class ChunkCache
cache.invalidateAll();
}
- public RebuffererFactory wrap(ChunkReader file)
+ private RebuffererFactory wrap(ChunkReader file)
{
return new CachingRebufferer(file);
}
diff --git a/src/java/org/apache/cassandra/cache/InstrumentingCache.java b/src/java/org/apache/cassandra/cache/InstrumentingCache.java
index e28766f..5d9eaac 100644
--- a/src/java/org/apache/cassandra/cache/InstrumentingCache.java
+++ b/src/java/org/apache/cassandra/cache/InstrumentingCache.java
@@ -96,6 +96,9 @@ public class InstrumentingCache<K, V>
public void clear()
{
map.clear();
+
+ // this does not clear metered metrics which are defined statically. for testing purposes, these can be
+ // cleared by CacheMetrics.reset()
metrics = new CacheMetrics(type, map);
}
diff --git a/src/java/org/apache/cassandra/io/util/FileHandle.java b/src/java/org/apache/cassandra/io/util/FileHandle.java
index b705769..6d3ae7c 100644
--- a/src/java/org/apache/cassandra/io/util/FileHandle.java
+++ b/src/java/org/apache/cassandra/io/util/FileHandle.java
@@ -419,7 +419,7 @@ public class FileHandle extends SharedCloseableImpl
private RebuffererFactory maybeCached(ChunkReader reader)
{
if (chunkCache != null && chunkCache.capacity() > 0)
- return chunkCache.wrap(reader);
+ return chunkCache.maybeWrap(reader);
return reader;
}
diff --git a/src/java/org/apache/cassandra/metrics/CacheMetrics.java b/src/java/org/apache/cassandra/metrics/CacheMetrics.java
index d4a00aa..92f7352 100644
--- a/src/java/org/apache/cassandra/metrics/CacheMetrics.java
+++ b/src/java/org/apache/cassandra/metrics/CacheMetrics.java
@@ -19,6 +19,8 @@ package org.apache.cassandra.metrics;
import java.util.function.DoubleSupplier;
+import com.google.common.annotations.VisibleForTesting;
+
import com.codahale.metrics.*;
import org.apache.cassandra.cache.CacheSize;
@@ -86,6 +88,13 @@ public class CacheMetrics
ratioGauge(hits::getFifteenMinuteRate, requests::getFifteenMinuteRate));
}
+ @VisibleForTesting
+ public void reset()
+ {
+ hits.mark(-hits.getCount());
+ misses.mark(-misses.getCount());
+ }
+
private static Metered sumMeters(Metered first, Metered second)
{
return new Metered()
diff --git a/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java b/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java
index a3a6928..6f12a47 100644
--- a/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java
+++ b/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java
@@ -20,8 +20,6 @@ package org.apache.cassandra.metrics;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nonnull;
-import com.google.common.annotations.VisibleForTesting;
-
import com.codahale.metrics.Timer;
import com.github.benmanes.caffeine.cache.stats.CacheStats;
import com.github.benmanes.caffeine.cache.stats.StatsCounter;
@@ -82,11 +80,4 @@ public class ChunkCacheMetrics extends CacheMetrics implements StatsCounter
{
return new CacheStats(hits.getCount(), misses.getCount(), missLatency.getCount(), 0L, missLatency.getCount(), 0L, 0L);
}
-
- @VisibleForTesting
- public void reset()
- {
- hits.mark(-hits.getCount());
- misses.mark(-misses.getCount());
- }
}
diff --git a/test/long/org/apache/cassandra/cql3/CachingBench.java b/test/long/org/apache/cassandra/cql3/CachingBench.java
index 0a6657f..f5f9ada 100644
--- a/test/long/org/apache/cassandra/cql3/CachingBench.java
+++ b/test/long/org/apache/cassandra/cql3/CachingBench.java
@@ -48,6 +48,8 @@ import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.utils.FBUtilities;
+import static org.assertj.core.api.Assertions.assertThat;
+
public class CachingBench extends CQLTester
{
private static final String STRATEGY = "LeveledCompactionStrategy";
@@ -240,9 +242,14 @@ public class CachingBench extends CQLTester
ChunkCache.instance.metrics.hitRate.getValue());
else
{
- Assert.assertTrue("Chunk cache had requests: " + ChunkCache.instance.metrics.requests.getCount(), ChunkCache.instance.metrics.requests.getCount() < COUNT);
+ assertThat(ChunkCache.instance.metrics.requests.getCount()).as("Chunk cache had requests: %s",
+ ChunkCache.instance.metrics.requests.getCount())
+ .isLessThan(COUNT);
System.out.println("Cache disabled");
}
+
+ assertThat(ChunkCache.instance.metrics.missLatency.getCount()).isGreaterThan(0);
+
System.out.println(String.format("Operations completed in %.3fs", (onEndTime - onStartTime) * 1e-3));
if (!CONCURRENT_COMPACTIONS)
System.out.println(String.format(", out of which %.3f for non-concurrent compaction", compactionTimeNanos * 1e-9));
diff --git a/test/unit/org/apache/cassandra/metrics/CacheMetricsTest.java b/test/unit/org/apache/cassandra/metrics/CacheMetricsTest.java
new file mode 100644
index 0000000..b2bf6af
--- /dev/null
+++ b/test/unit/org/apache/cassandra/metrics/CacheMetricsTest.java
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.metrics;
+
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.apache.cassandra.OrderedJUnit4ClassRunner;
+import org.apache.cassandra.cache.CacheSize;
+import org.apache.cassandra.cache.ICache;
+import org.apache.cassandra.cache.InstrumentingCache;
+
+import static org.junit.Assert.assertEquals;
+
+@RunWith(OrderedJUnit4ClassRunner.class)
+public class CacheMetricsTest
+{
+ private static final long capacity = 65536;
+
+ @Test
+ public void testCacheMetrics()
+ {
+ ICache<String,Object> mockedCache = new MapMockedCache();
+ InstrumentingCache<String,Object> cache = new InstrumentingCache<>("cache", mockedCache);
+ CacheMetrics metrics = cache.getMetrics();
+
+ assertCacheMetrics(metrics, expect(mockedCache));
+
+ cache.put("k1", "v1");
+ assertCacheMetrics(metrics, expect(mockedCache));
+
+ getFromCache(cache, "k1", 10);
+ assertCacheMetrics(metrics, expect(mockedCache).hits(10).misses(0));
+
+ getFromCache(cache, "k2", 10);
+ assertCacheMetrics(metrics, expect(mockedCache).hits(10).misses(10));
+
+ cache.put("k2", "v2");
+ getFromCache(cache, "k2", 70);
+ getFromCache(cache, "k3", 10);
+ assertCacheMetrics(metrics, expect(mockedCache).hits(80).misses(20));
+
+ cache.clear();
+ metrics.reset();
+ assertCacheMetrics(metrics, expect(mockedCache));
+ }
+
+ private void getFromCache(InstrumentingCache<String,Object> cache, String key, int times)
+ {
+ for (int ix = 0; ix < times; ix++)
+ {
+ cache.get(key);
+ }
+ }
+
+ private void assertCacheMetrics(CacheMetrics actual, CacheMetricsExpectation expectation)
+ {
+ // assuming meters/guagues (hits, misses, requests, and hitRate) will have correct one/five/fifteenMinute
+ // calculations - applying some general assertions for hitRate calculations that essentially just smoke test
+ // existence (i.e. NaN at initialization) since they are established by way of an inner class on CacheMetrics
+ // itself.
+ assertEquals(expectation.cacheSize.capacity(), actual.capacity.getValue().longValue());
+ assertEquals(expectation.cacheSize.weightedSize(), actual.size.getValue().longValue());
+ assertEquals(expectation.cacheSize.size(), actual.entries.getValue().intValue());
+ assertEquals(expectation.hits, actual.hits.getCount());
+ assertEquals(expectation.misses, actual.misses.getCount());
+ assertEquals(expectation.requests(), actual.requests.getCount());
+ assertEquals(expectation.hitRate(), actual.hitRate.getValue(), 0.001d);
+ assertEquals(Double.NaN, actual.oneMinuteHitRate.getValue(), 0.001d);
+ assertEquals(Double.NaN, actual.fiveMinuteHitRate.getValue(), 0.001d);
+ assertEquals(Double.NaN, actual.fifteenMinuteHitRate.getValue(), 0.001d);
+ }
+
+ static CacheMetricsExpectation expect(CacheSize cacheSize)
+ {
+ return new CacheMetricsExpectation(cacheSize);
+ }
+
+ private static class MapMockedCache implements ICache<String,Object>
+ {
+ private final Map<String,Object> map = new HashMap<>();
+
+ public void put(String key, Object value)
+ {
+ map.put(key, value);
+ }
+
+ public boolean putIfAbsent(String key, Object value)
+ {
+ return map.putIfAbsent(key, value) == null;
+ }
+
+ public boolean replace(String key, Object old, Object value)
+ {
+ return map.replace(key, old, value);
+ }
+
+ public Object get(String key)
+ {
+ return map.get(key);
+ }
+
+ public void remove(String key)
+ {
+ map.remove(key);
+ }
+
+ public void clear()
+ {
+ map.clear();
+ }
+
+ public Iterator<String> keyIterator()
+ {
+ return map.keySet().iterator();
+ }
+
+ public Iterator<String> hotKeyIterator(int n)
+ {
+ return map.keySet().iterator();
+ }
+
+ public boolean containsKey(String key)
+ {
+ return map.containsKey(key);
+ }
+
+ public long capacity()
+ {
+ // capacity in bytes but just using a fixed number here since since the validation is just to ensure
+ // that this number is equivalent to the metric that publishes it.
+ return capacity;
+ }
+
+ public void setCapacity(long capacity)
+ {
+ throw new UnsupportedOperationException("Not needed for testing");
+ }
+
+ public int size()
+ {
+ return map.size();
+ }
+
+ public long weightedSize()
+ {
+ // should be cache size in bytes, but no need to calculate that for tests since the validation
+ // is just to ensure that this number is equivalent to the metric that publishes it.
+ return map.size() * 8;
+ }
+ }
+
+ private static class CacheMetricsExpectation
+ {
+ final CacheSize cacheSize;
+ private long hits = 0;
+ private long misses = 0;
+
+ CacheMetricsExpectation(CacheSize cacheSize)
+ {
+ this.cacheSize = cacheSize;
+ }
+
+ public CacheMetricsExpectation hits(long hits)
+ {
+ this.hits = hits;
+ return this;
+ }
+
+ public CacheMetricsExpectation misses(long misses)
+ {
+ this.misses = misses;
+ return this;
+ }
+
+ public long requests()
+ {
+ return hits + misses;
+ }
+
+ public double hitRate()
+ {
+ return requests() == 0 ? Double.NaN : ((double) hits / (double) requests());
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org