Repository: cassandra
Updated Branches:
  refs/heads/trunk 8b9515bd2 -> e6b8e7a72


Fix up chunk cache handling of metrics

patch by Aleksey Yeschenko; reviewed by Benedict Elliott Smith for
CASSANDRA-14628


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/e6b8e7a7
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/e6b8e7a7
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/e6b8e7a7

Branch: refs/heads/trunk
Commit: e6b8e7a72f783ed0e1b5a2c04381f89b533229a4
Parents: 8b9515b
Author: Aleksey Yeshchenko <[email protected]>
Authored: Wed Aug 8 15:17:26 2018 +0100
Committer: Aleksey Yeshchenko <[email protected]>
Committed: Tue Aug 21 12:33:42 2018 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../org/apache/cassandra/cache/ChunkCache.java  |  36 +++---
 .../cassandra/cache/InstrumentingCache.java     |   3 +-
 .../apache/cassandra/metrics/CacheMetrics.java  | 116 +++++++++++--------
 .../cassandra/metrics/CacheMissMetrics.java     | 114 ------------------
 .../cassandra/metrics/ChunkCacheMetrics.java    |  92 +++++++++++++++
 6 files changed, 179 insertions(+), 183 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/e6b8e7a7/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index d906879..5fa28f5 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.0
+ * Fix up chunk cache handling of metrics (CASSANDRA-14628)
  * Extend IAuthenticator to accept peer SSL certificates (CASSANDRA-14652)
  * Incomplete handling of exceptions when decoding incoming messages 
(CASSANDRA-14574)
  * Add diagnostic events for user audit logging (CASSANDRA-13668)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e6b8e7a7/src/java/org/apache/cassandra/cache/ChunkCache.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cache/ChunkCache.java 
b/src/java/org/apache/cassandra/cache/ChunkCache.java
index 9284377..0edb681 100644
--- a/src/java/org/apache/cassandra/cache/ChunkCache.java
+++ b/src/java/org/apache/cassandra/cache/ChunkCache.java
@@ -29,11 +29,10 @@ import com.google.common.collect.Iterables;
 import com.google.common.util.concurrent.MoreExecutors;
 
 import com.github.benmanes.caffeine.cache.*;
-import com.codahale.metrics.Timer;
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.io.sstable.CorruptSSTableException;
 import org.apache.cassandra.io.util.*;
-import org.apache.cassandra.metrics.CacheMissMetrics;
+import org.apache.cassandra.metrics.ChunkCacheMetrics;
 import org.apache.cassandra.utils.memory.BufferPool;
 
 public class ChunkCache
@@ -47,7 +46,7 @@ public class ChunkCache
     public static final ChunkCache instance = enabled ? new ChunkCache() : 
null;
 
     private final LoadingCache<Key, Buffer> cache;
-    public final CacheMissMetrics metrics;
+    public final ChunkCacheMetrics metrics;
 
     static class Key
     {
@@ -135,29 +134,25 @@ public class ChunkCache
         }
     }
 
-    public ChunkCache()
+    private ChunkCache()
     {
+        metrics = new ChunkCacheMetrics(this);
         cache = Caffeine.newBuilder()
-                .maximumWeight(cacheSize)
-                .executor(MoreExecutors.directExecutor())
-                .weigher((key, buffer) -> ((Buffer) buffer).buffer.capacity())
-                .removalListener(this)
-                .build(this);
-        metrics = new CacheMissMetrics("ChunkCache", this);
+                        .maximumWeight(cacheSize)
+                        .executor(MoreExecutors.directExecutor())
+                        .weigher((key, buffer) -> ((Buffer) 
buffer).buffer.capacity())
+                        .removalListener(this)
+                        .recordStats(() -> metrics)
+                        .build(this);
     }
 
     @Override
-    public Buffer load(Key key) throws Exception
+    public Buffer load(Key key)
     {
-        ChunkReader rebufferer = key.file;
-        metrics.misses.mark();
-        try (Timer.Context ctx = metrics.missLatency.time())
-        {
-            ByteBuffer buffer = BufferPool.get(key.file.chunkSize(), 
key.file.preferredBufferType());
-            assert buffer != null;
-            rebufferer.readChunk(key.position, buffer);
-            return new Buffer(buffer, key.position);
-        }
+        ByteBuffer buffer = BufferPool.get(key.file.chunkSize(), 
key.file.preferredBufferType());
+        assert buffer != null;
+        key.file.readChunk(key.position, buffer);
+        return new Buffer(buffer, key.position);
     }
 
     @Override
@@ -229,7 +224,6 @@ public class ChunkCache
         {
             try
             {
-                metrics.requests.mark();
                 long pageAlignedPos = position & alignmentMask;
                 Buffer buf;
                 do

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e6b8e7a7/src/java/org/apache/cassandra/cache/InstrumentingCache.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cache/InstrumentingCache.java 
b/src/java/org/apache/cassandra/cache/InstrumentingCache.java
index c8728fd..e28766f 100644
--- a/src/java/org/apache/cassandra/cache/InstrumentingCache.java
+++ b/src/java/org/apache/cassandra/cache/InstrumentingCache.java
@@ -56,9 +56,10 @@ public class InstrumentingCache<K, V>
     public V get(K key)
     {
         V v = map.get(key);
-        metrics.requests.mark();
         if (v != null)
             metrics.hits.mark();
+        else
+            metrics.misses.mark();
         return v;
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e6b8e7a7/src/java/org/apache/cassandra/metrics/CacheMetrics.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/metrics/CacheMetrics.java 
b/src/java/org/apache/cassandra/metrics/CacheMetrics.java
index e623dcb..d4a00aa 100644
--- a/src/java/org/apache/cassandra/metrics/CacheMetrics.java
+++ b/src/java/org/apache/cassandra/metrics/CacheMetrics.java
@@ -17,10 +17,10 @@
  */
 package org.apache.cassandra.metrics;
 
-import com.codahale.metrics.Gauge;
-import com.codahale.metrics.Meter;
-import com.codahale.metrics.RatioGauge;
-import org.apache.cassandra.cache.ICache;
+import java.util.function.DoubleSupplier;
+
+import com.codahale.metrics.*;
+import org.apache.cassandra.cache.CacheSize;
 
 import static org.apache.cassandra.metrics.CassandraMetricsRegistry.Metrics;
 
@@ -31,10 +31,18 @@ public class CacheMetrics
 {
     /** Cache capacity in bytes */
     public final Gauge<Long> capacity;
+    /** Total size of cache, in bytes */
+    public final Gauge<Long> size;
+    /** Total number of cache entries */
+    public final Gauge<Integer> entries;
+
     /** Total number of cache hits */
     public final Meter hits;
+    /** Total number of cache misses */
+    public final Meter misses;
     /** Total number of cache requests */
-    public final Meter requests;
+    public final Metered requests;
+
     /** all time cache hit rate */
     public final Gauge<Double> hitRate;
     /** 1m hit rate */
@@ -43,10 +51,8 @@ public class CacheMetrics
     public final Gauge<Double> fiveMinuteHitRate;
     /** 15m hit rate */
     public final Gauge<Double> fifteenMinuteHitRate;
-    /** Total size of cache, in bytes */
-    public final Gauge<Long> size;
-    /** Total number of cache entries */
-    public final Gauge<Integer> entries;
+
+    protected final MetricNameFactory factory;
 
     /**
      * Create metrics for given cache.
@@ -54,61 +60,77 @@ public class CacheMetrics
      * @param type Type of Cache to identify metrics.
      * @param cache Cache to measure metrics
      */
-    public CacheMetrics(String type, final ICache<?, ?> cache)
+    public CacheMetrics(String type, CacheSize cache)
     {
-        MetricNameFactory factory = new DefaultNameFactory("Cache", type);
+        factory = new DefaultNameFactory("Cache", type);
+
+        capacity = Metrics.register(factory.createMetricName("Capacity"), 
cache::capacity);
+        size = Metrics.register(factory.createMetricName("Size"), 
cache::weightedSize);
+        entries = Metrics.register(factory.createMetricName("Entries"), 
cache::size);
 
-        capacity = Metrics.register(factory.createMetricName("Capacity"), new 
Gauge<Long>()
-        {
-            public Long getValue()
-            {
-                return cache.capacity();
-            }
-        });
         hits = Metrics.meter(factory.createMetricName("Hits"));
-        requests = Metrics.meter(factory.createMetricName("Requests"));
-        hitRate = Metrics.register(factory.createMetricName("HitRate"), new 
RatioGauge()
+        misses = Metrics.meter(factory.createMetricName("Misses"));
+        requests = Metrics.register(factory.createMetricName("Requests"), 
sumMeters(hits, misses));
+
+        hitRate =
+            Metrics.register(factory.createMetricName("HitRate"),
+                             ratioGauge(hits::getCount, requests::getCount));
+        oneMinuteHitRate =
+            Metrics.register(factory.createMetricName("OneMinuteHitRate"),
+                             ratioGauge(hits::getOneMinuteRate, 
requests::getOneMinuteRate));
+        fiveMinuteHitRate =
+            Metrics.register(factory.createMetricName("FiveMinuteHitRate"),
+                             ratioGauge(hits::getFiveMinuteRate, 
requests::getFiveMinuteRate));
+        fifteenMinuteHitRate =
+            Metrics.register(factory.createMetricName("FifteenMinuteHitRate"),
+                             ratioGauge(hits::getFifteenMinuteRate, 
requests::getFifteenMinuteRate));
+    }
+
+    private static Metered sumMeters(Metered first, Metered second)
+    {
+        return new Metered()
         {
             @Override
-            public Ratio getRatio()
+            public long getCount()
             {
-                return Ratio.of(hits.getCount(), requests.getCount());
+                return first.getCount() + second.getCount();
             }
-        });
-        oneMinuteHitRate = 
Metrics.register(factory.createMetricName("OneMinuteHitRate"), new RatioGauge()
-        {
-            protected Ratio getRatio()
+
+            @Override
+            public double getMeanRate()
             {
-                return Ratio.of(hits.getOneMinuteRate(), 
requests.getOneMinuteRate());
+                return first.getMeanRate() + second.getMeanRate();
             }
-        });
-        fiveMinuteHitRate = 
Metrics.register(factory.createMetricName("FiveMinuteHitRate"), new RatioGauge()
-        {
-            protected Ratio getRatio()
+
+            @Override
+            public double getOneMinuteRate()
             {
-                return Ratio.of(hits.getFiveMinuteRate(), 
requests.getFiveMinuteRate());
+                return first.getOneMinuteRate() + second.getOneMinuteRate();
             }
-        });
-        fifteenMinuteHitRate = 
Metrics.register(factory.createMetricName("FifteenMinuteHitRate"), new 
RatioGauge()
-        {
-            protected Ratio getRatio()
+
+            @Override
+            public double getFiveMinuteRate()
             {
-                return Ratio.of(hits.getFifteenMinuteRate(), 
requests.getFifteenMinuteRate());
+                return first.getFiveMinuteRate() + second.getFiveMinuteRate();
             }
-        });
-        size = Metrics.register(factory.createMetricName("Size"), new 
Gauge<Long>()
-        {
-            public Long getValue()
+
+            @Override
+            public double getFifteenMinuteRate()
             {
-                return cache.weightedSize();
+                return first.getFifteenMinuteRate() + 
second.getFifteenMinuteRate();
             }
-        });
-        entries = Metrics.register(factory.createMetricName("Entries"), new 
Gauge<Integer>()
+        };
+    }
+
+    private static RatioGauge ratioGauge(DoubleSupplier numeratorSupplier, 
DoubleSupplier denominatorSupplier)
+    {
+        return new RatioGauge()
         {
-            public Integer getValue()
+            @Override
+            public Ratio getRatio()
             {
-                return cache.size();
+                return Ratio.of(numeratorSupplier.getAsDouble(), 
denominatorSupplier.getAsDouble());
             }
-        });
+        };
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e6b8e7a7/src/java/org/apache/cassandra/metrics/CacheMissMetrics.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/metrics/CacheMissMetrics.java 
b/src/java/org/apache/cassandra/metrics/CacheMissMetrics.java
deleted file mode 100644
index 19d61ef..0000000
--- a/src/java/org/apache/cassandra/metrics/CacheMissMetrics.java
+++ /dev/null
@@ -1,114 +0,0 @@
-/*
- * 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 com.codahale.metrics.Gauge;
-import com.codahale.metrics.Meter;
-import com.codahale.metrics.RatioGauge;
-import com.codahale.metrics.Timer;
-import org.apache.cassandra.cache.CacheSize;
-
-import static org.apache.cassandra.metrics.CassandraMetricsRegistry.Metrics;
-
-/**
- * Metrics for {@code ICache}.
- */
-public class CacheMissMetrics
-{
-    /** Cache capacity in bytes */
-    public final Gauge<Long> capacity;
-    /** Total number of cache hits */
-    public final Meter misses;
-    /** Total number of cache requests */
-    public final Meter requests;
-    /** Latency of misses */
-    public final Timer missLatency;
-    /** all time cache hit rate */
-    public final Gauge<Double> hitRate;
-    /** 1m hit rate */
-    public final Gauge<Double> oneMinuteHitRate;
-    /** 5m hit rate */
-    public final Gauge<Double> fiveMinuteHitRate;
-    /** 15m hit rate */
-    public final Gauge<Double> fifteenMinuteHitRate;
-    /** Total size of cache, in bytes */
-    public final Gauge<Long> size;
-    /** Total number of cache entries */
-    public final Gauge<Integer> entries;
-
-    /**
-     * Create metrics for given cache.
-     *
-     * @param type Type of Cache to identify metrics.
-     * @param cache Cache to measure metrics
-     */
-    public CacheMissMetrics(String type, final CacheSize cache)
-    {
-        MetricNameFactory factory = new DefaultNameFactory("Cache", type);
-
-        capacity = Metrics.register(factory.createMetricName("Capacity"), 
(Gauge<Long>) cache::capacity);
-        misses = Metrics.meter(factory.createMetricName("Misses"));
-        requests = Metrics.meter(factory.createMetricName("Requests"));
-        missLatency = Metrics.timer(factory.createMetricName("MissLatency"));
-        hitRate = Metrics.register(factory.createMetricName("HitRate"), new 
RatioGauge()
-        {
-            @Override
-            public Ratio getRatio()
-            {
-                long req = requests.getCount();
-                long mis = misses.getCount();
-                return Ratio.of(req - mis, req);
-            }
-        });
-        oneMinuteHitRate = 
Metrics.register(factory.createMetricName("OneMinuteHitRate"), new RatioGauge()
-        {
-            protected Ratio getRatio()
-            {
-                double req = requests.getOneMinuteRate();
-                double mis = misses.getOneMinuteRate();
-                return Ratio.of(req - mis, req);
-            }
-        });
-        fiveMinuteHitRate = 
Metrics.register(factory.createMetricName("FiveMinuteHitRate"), new RatioGauge()
-        {
-            protected Ratio getRatio()
-            {
-                double req = requests.getFiveMinuteRate();
-                double mis = misses.getFiveMinuteRate();
-                return Ratio.of(req - mis, req);
-            }
-        });
-        fifteenMinuteHitRate = 
Metrics.register(factory.createMetricName("FifteenMinuteHitRate"), new 
RatioGauge()
-        {
-            protected Ratio getRatio()
-            {
-                double req = requests.getFifteenMinuteRate();
-                double mis = misses.getFifteenMinuteRate();
-                return Ratio.of(req - mis, req);
-            }
-        });
-        size = Metrics.register(factory.createMetricName("Size"), 
(Gauge<Long>) cache::weightedSize);
-        entries = Metrics.register(factory.createMetricName("Entries"), 
(Gauge<Integer>) cache::size);
-    }
-
-    public void reset()
-    {
-        requests.mark(-requests.getCount());
-        misses.mark(-misses.getCount());
-    }
-}

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e6b8e7a7/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java 
b/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java
new file mode 100644
index 0000000..a3a6928
--- /dev/null
+++ b/src/java/org/apache/cassandra/metrics/ChunkCacheMetrics.java
@@ -0,0 +1,92 @@
+/*
+ * 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.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;
+import org.apache.cassandra.cache.ChunkCache;
+
+import static org.apache.cassandra.metrics.CassandraMetricsRegistry.Metrics;
+
+/**
+ * Metrics for {@code ICache}.
+ */
+public class ChunkCacheMetrics extends CacheMetrics implements StatsCounter
+{
+    /** Latency of misses */
+    public final Timer missLatency;
+
+    /**
+     * Create metrics for the provided chunk cache.
+     *
+     * @param cache Chunk cache to measure metrics
+     */
+    public ChunkCacheMetrics(ChunkCache cache)
+    {
+        super("ChunkCache", cache);
+        missLatency = Metrics.timer(factory.createMetricName("MissLatency"));
+    }
+
+    @Override
+    public void recordHits(int count)
+    {
+        hits.mark(count);
+    }
+
+    @Override
+    public void recordMisses(int count)
+    {
+        misses.mark(count);
+    }
+
+    @Override
+    public void recordLoadSuccess(long loadTime)
+    {
+        missLatency.update(loadTime, TimeUnit.NANOSECONDS);
+    }
+
+    @Override
+    public void recordLoadFailure(long loadTime)
+    {
+    }
+
+    @Override
+    public void recordEviction()
+    {
+    }
+
+    @Nonnull
+    @Override
+    public CacheStats snapshot()
+    {
+        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());
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to