Add cache keys index metrics
The following metrics were added foe each cache:
* cache/chroniclemap/keys_index_size_<cache-name> for showing the
current size of the index (that one is one to one with disk entries)
* cache/chroniclemap/keys_index_add_latency_<cache-name> to indicate how
much time it takes to add key to the index over time
* cache/chroniclemap/keys_index_remove_and_consume_older_than_latency_<cache-name>
to indicate how much time it takes to remove and consume all keys
older than expiration time
* cache/chroniclemap/keys_index_remove_lru_key_latency_<cache-name> to
indicate how much time it takes to remove and consumer LRU key
Bug: Issue 15121
Change-Id: I8141c0f00dfe6e7dc3b06517c3383d19fe883492
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java
index 34e3eac..a6f608d 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndex.java
@@ -18,6 +18,10 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.errorprone.annotations.CompatibleWith;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.Objects;
@@ -63,10 +67,62 @@
}
}
- private final Set<TimedKey> keys;
+ private class Metrics {
+ private final Timer0 addLatency;
+ private final Timer0 removeAndConsumeOlderThanLatency;
+ private final Timer0 removeAndConsumeLruKeyLatency;
- CacheKeysIndex() {
+ private Metrics(MetricMaker metricMaker, String name) {
+ String sanitizedName = metricMaker.sanitizeMetricName(name);
+
+ metricMaker.newCallbackMetric(
+ "cache/chroniclemap/keys_index_size_" + sanitizedName,
+ Integer.class,
+ new Description(
+ String.format(
+ "The number of cache index keys for %s cache that are currently in memory",
+ name)),
+ keys::size);
+
+ addLatency =
+ metricMaker.newTimer(
+ "cache/chroniclemap/keys_index_add_latency_" + sanitizedName,
+ new Description(
+ String.format("The latency of adding key to the index for %s cache", name))
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+
+ removeAndConsumeOlderThanLatency =
+ metricMaker.newTimer(
+ "cache/chroniclemap/keys_index_remove_and_consume_older_than_latency_"
+ + sanitizedName,
+ new Description(
+ String.format(
+ "The latency of removing and consuming all keys older than expiration"
+ + " time for the index for %s cache",
+ name))
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+
+ removeAndConsumeLruKeyLatency =
+ metricMaker.newTimer(
+ "cache/chroniclemap/keys_index_remove_lru_key_latency_" + sanitizedName,
+ new Description(
+ String.format(
+ "The latency of removing and consuming LRU key from the index for %s"
+ + " cache",
+ name))
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+ }
+ }
+
+ private final Set<TimedKey> keys;
+ private final Metrics metrics;
+
+ CacheKeysIndex(MetricMaker metricMaker, String name) {
this.keys = Collections.synchronizedSet(new LinkedHashSet<>());
+ this.metrics = new Metrics(metricMaker, name);
}
@SuppressWarnings("unchecked")
@@ -75,8 +131,10 @@
TimedKey timedKey = new TimedKey((T) key, created);
// bubble up MRU key by re-adding it to a set
- keys.remove(timedKey);
- keys.add(timedKey);
+ try (Timer0.Context timer = metrics.addLatency.start()) {
+ keys.remove(timedKey);
+ keys.add(timedKey);
+ }
}
void refresh(T key) {
@@ -84,32 +142,36 @@
}
void removeAndConsumeKeysOlderThan(long time, Consumer<T> consumer) {
- Set<TimedKey> toRemoveAndConsume;
- synchronized (keys) {
- toRemoveAndConsume =
- keys.stream().filter(key -> key.created < time).collect(toUnmodifiableSet());
+ try (Timer0.Context timer = metrics.removeAndConsumeOlderThanLatency.start()) {
+ Set<TimedKey> toRemoveAndConsume;
+ synchronized (keys) {
+ toRemoveAndConsume =
+ keys.stream().filter(key -> key.created < time).collect(toUnmodifiableSet());
+ }
+ toRemoveAndConsume.forEach(
+ key -> {
+ keys.remove(key);
+ consumer.accept(key.getKey());
+ });
}
- toRemoveAndConsume.forEach(
- key -> {
- keys.remove(key);
- consumer.accept(key.getKey());
- });
}
boolean removeAndConsumeLruKey(Consumer<T> consumer) {
- Optional<TimedKey> lruKey;
- synchronized (keys) {
- lruKey = keys.stream().findFirst();
- }
+ try (Timer0.Context timer = metrics.removeAndConsumeLruKeyLatency.start()) {
+ Optional<TimedKey> lruKey;
+ synchronized (keys) {
+ lruKey = keys.stream().findFirst();
+ }
- return lruKey
- .map(
- key -> {
- keys.remove(key);
- consumer.accept(key.getKey());
- return true;
- })
- .orElse(false);
+ return lruKey
+ .map(
+ key -> {
+ keys.remove(key);
+ consumer.accept(key.getKey());
+ return true;
+ })
+ .orElse(false);
+ }
}
void invalidate(@CompatibleWith("T") Object key) {
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java
index 98d7f42..1161699 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheFactory.java
@@ -117,7 +117,11 @@
cache =
new ChronicleMapCacheImpl<>(
- in, config, memLoader, new InMemoryCacheLoadingFromStoreImpl<>(mem, false));
+ in,
+ config,
+ metricMaker,
+ memLoader,
+ new InMemoryCacheLoadingFromStoreImpl<>(mem, false));
} catch (IOException e) {
throw new UncheckedIOException(e);
@@ -165,7 +169,11 @@
cache =
new ChronicleMapCacheImpl<>(
- in, config, memLoader, new InMemoryCacheLoadingFromStoreImpl<>(mem, true));
+ in,
+ config,
+ metricMaker,
+ memLoader,
+ new InMemoryCacheLoadingFromStoreImpl<>(mem, true));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
index 12a463b..3740959 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheImpl.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.googlesource.gerrit.modules.cache.chroniclemap;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.AbstractLoadingCache;
import com.google.common.cache.CacheStats;
import com.google.common.flogger.FluentLogger;
@@ -27,7 +26,6 @@
import java.sql.Timestamp;
import java.time.Duration;
import java.time.Instant;
-import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.LongAdder;
@@ -57,7 +55,7 @@
this.cacheDefinition = def;
this.config = config;
- this.keysIndex = new CacheKeysIndex<>();
+ this.keysIndex = new CacheKeysIndex<>(metricMaker, def.name());
this.store = createOrRecoverStore(def, config, metricMaker);
this.memLoader =
new ChronicleMapCacheLoader<>(
@@ -68,12 +66,13 @@
ChronicleMapCacheImpl(
PersistentCacheDef<K, V> def,
ChronicleMapCacheConfig config,
+ MetricMaker metricMaker,
ChronicleMapCacheLoader<K, V> memLoader,
InMemoryCache<K, V> mem) {
this.cacheDefinition = def;
this.config = config;
- this.keysIndex = new CacheKeysIndex<>();
+ this.keysIndex = new CacheKeysIndex<>(metricMaker, def.name());
this.memLoader = memLoader;
this.mem = mem;
this.store = memLoader.getStore();
@@ -342,9 +341,4 @@
public String name() {
return store.name();
}
-
- @VisibleForTesting
- Set<CacheKeysIndex<K>.TimedKey> keys() {
- return keysIndex.keys();
- }
}
diff --git a/src/main/resources/Documentation/metrics.md b/src/main/resources/Documentation/metrics.md
index 565fe62..b0bc902 100644
--- a/src/main/resources/Documentation/metrics.md
+++ b/src/main/resources/Documentation/metrics.md
@@ -20,5 +20,17 @@
* cache/chroniclemap/max_autoresizes_<cache-name>
: The maximum number of times the cache can automatically expand its capacity.
+* cache/chroniclemap/keys_index_size_<cache-name>
+ : The number of index keys for the cache that are currently in memory.
+
+* cache/chroniclemap/keys_index_add_latency_<cache-name>
+ : The latency of adding cache key to an index.
+
+* cache/chroniclemap/keys_index_remove_and_consume_older_than_latency_<cache-name>
+ : The latency of removing and consuming all keys older than expiration time for an index.
+
+* cache/chroniclemap/keys_index_remove_lru_key_latency_<cache-name>
+ : The latency of removing and consuming LRU key from an index.
+
* "cache/chroniclemap/store_put_failures_<cache-name>
: The number of errors caught when inserting entries in chronicle-map store
\ No newline at end of file
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java
index 405e5a5..27c9e76 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/CacheKeysIndexTest.java
@@ -20,6 +20,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
+import com.google.gerrit.metrics.DisabledMetricMaker;
import java.util.List;
import java.util.function.Consumer;
import org.junit.Before;
@@ -30,7 +31,7 @@
@Before
public void setup() {
- index = new CacheKeysIndex<>();
+ index = new CacheKeysIndex<>(new DisabledMetricMaker(), "test-cache");
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
index ecc457a..9c839f0 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/ChronicleMapCacheTest.java
@@ -20,6 +20,7 @@
import com.codahale.metrics.Counter;
import com.codahale.metrics.Gauge;
import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.Timer;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.acceptance.WaitUtil;
@@ -455,22 +456,69 @@
}
@Test
- public void shouldResetKeysIndexWhenInvalidateAll() throws Exception {
+ public void shouldTriggerKeysIndexSizeCacheMetric() throws Exception {
String cachedValue = UUID.randomUUID().toString();
int maxEntries = 10;
- int maxHotKeyCapacity = 3;
+ int expectedKeysSize = 3;
+ final Duration METRIC_TRIGGER_TIMEOUT = Duration.ofSeconds(2);
+ String keysIndexSizeMetricName = "cache/chroniclemap/keys_index_size_" + testCacheName;
gerritConfig.setInt("cache", testCacheName, "maxEntries", maxEntries);
gerritConfig.save();
ChronicleMapCacheImpl<String, String> cache = newCacheWithMetrics(testCacheName, cachedValue);
- for (int i = 0; i < maxHotKeyCapacity; i++) {
+ assertThat(getMetric(keysIndexSizeMetricName).getValue()).isEqualTo(0);
+
+ for (int i = 0; i < expectedKeysSize; i++) {
cache.put(cachedValue + i, cachedValue);
}
- assertThat(cache.keys()).isNotEmpty();
+ WaitUtil.waitUntil(
+ () -> (int) getMetric(keysIndexSizeMetricName).getValue() == expectedKeysSize,
+ METRIC_TRIGGER_TIMEOUT);
+ }
+
+ @Test
+ public void shouldTriggerKeysIndexAddLatencyCacheMetric() throws Exception {
+ int maxEntries = 10;
+ final Duration METRIC_TRIGGER_TIMEOUT = Duration.ofSeconds(2);
+ String keysIndexAddLatencyMetricName =
+ "cache/chroniclemap/keys_index_add_latency_" + testCacheName;
+ gerritConfig.setInt("cache", testCacheName, "maxEntries", maxEntries);
+ gerritConfig.save();
+
+ ChronicleMapCacheImpl<String, String> cache = newCacheWithMetrics(testCacheName, null);
+ assertThat(getTimer(keysIndexAddLatencyMetricName).getCount()).isEqualTo(0L);
+
+ String cachedValue = UUID.randomUUID().toString();
+ cache.put(cachedValue, cachedValue);
+
+ WaitUtil.waitUntil(
+ () -> getTimer(keysIndexAddLatencyMetricName).getCount() == 1L, METRIC_TRIGGER_TIMEOUT);
+ }
+
+ @Test
+ public void shouldResetKeysIndexWhenInvalidateAll() throws Exception {
+ String cachedValue = UUID.randomUUID().toString();
+ int maxEntries = 10;
+ int expectedKeysSize = 3;
+ final Duration METRIC_TRIGGER_TIMEOUT = Duration.ofSeconds(2);
+ String keysIndexSizeMetricName = "cache/chroniclemap/keys_index_size_" + testCacheName;
+ gerritConfig.setInt("cache", testCacheName, "maxEntries", maxEntries);
+ gerritConfig.save();
+
+ ChronicleMapCacheImpl<String, String> cache = newCacheWithMetrics(testCacheName, cachedValue);
+
+ for (int i = 0; i < expectedKeysSize; i++) {
+ cache.put(cachedValue + i, cachedValue);
+ }
+
+ WaitUtil.waitUntil(
+ () -> (int) getMetric(keysIndexSizeMetricName).getValue() == expectedKeysSize,
+ METRIC_TRIGGER_TIMEOUT);
cache.invalidateAll();
- assertThat(cache.keys()).isEmpty();
+ WaitUtil.waitUntil(
+ () -> (int) getMetric(keysIndexSizeMetricName).getValue() == 0, METRIC_TRIGGER_TIMEOUT);
}
@Test
@@ -480,12 +528,22 @@
String percentageFreeMetricName = "cache/chroniclemap/percentage_free_space_" + sanitized;
String autoResizeMetricName = "cache/chroniclemap/remaining_autoresizes_" + sanitized;
String maxAutoResizeMetricName = "cache/chroniclemap/max_autoresizes_" + sanitized;
+ String keysIndexSizeMetricName = "cache/chroniclemap/keys_index_size_" + sanitized;
+ String keysIndexAddLatencyMetricName = "cache/chroniclemap/keys_index_add_latency_" + sanitized;
+ String keysIndexRemoveOlderThanLatencyMetricName =
+ "cache/chroniclemap/keys_index_remove_and_consume_older_than_latency_" + sanitized;
+ String keysIndexRemoveLruLatencyMetricName =
+ "cache/chroniclemap/keys_index_remove_lru_key_latency_" + sanitized;
newCacheWithMetrics(cacheName, null);
getMetric(percentageFreeMetricName);
getMetric(autoResizeMetricName);
getMetric(maxAutoResizeMetricName);
+ getMetric(keysIndexSizeMetricName);
+ getTimer(keysIndexAddLatencyMetricName);
+ getTimer(keysIndexRemoveOlderThanLatencyMetricName);
+ getTimer(keysIndexRemoveLruLatencyMetricName);
}
private int valueSize(String value) {
@@ -588,4 +646,10 @@
assertWithMessage(name).that(counter).isNotNull();
return counter;
}
+
+ private Timer getTimer(String name) {
+ Timer timer = (Timer) metricRegistry.getMetrics().get(name);
+ assertWithMessage(name).that(timer).isNotNull();
+ return timer;
+ }
}
