Add read/write metrics to TimedValueMarshaller
The following metrics were added:
* cache/chroniclemap/store_serialize_latency_<cache-name> the latency of
serializing entries in chronicle-map store
* cache/chroniclemap/store_deserialize_latency_<cache-name> the latency
of deserializing entries in chronicle-map store
Notes:
* metrics are not serializable hence as such they have to be transient
* in case 'readResolve' method is used instead of constructor fallback
to metrics cache to get metrics; it is safe operation as store is
created only after the TimeValueMarshaller is created for a cache in
question hence metricsCache is going to have the corresponding entry
Change-Id: I195d83d4ebf351ad82925130d4e0c77231c3ec07
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 b9d5c29..1d60084 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
@@ -110,7 +110,7 @@
}
mapBuilder.averageValueSize(config.getAverageValueSize());
- mapBuilder.valueMarshaller(new TimedValueMarshaller<>(def.name()));
+ mapBuilder.valueMarshaller(new TimedValueMarshaller<>(metricMaker, def.name()));
mapBuilder.entries(config.getMaxEntries());
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/SerializationMetricsForCache.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/SerializationMetricsForCache.java
new file mode 100644
index 0000000..b49a65d
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/SerializationMetricsForCache.java
@@ -0,0 +1,82 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// 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
+//
+// 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 com.googlesource.gerrit.modules.cache.chroniclemap;
+
+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.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * Metrics contains relations with objects created upon Gerrit start and as such it cannot be
+ * serialized. Upon the situation when store is being restored from a persistent file Marshaller
+ * gets deserialised and its transient metrics value needs to be re-initialised with object that was
+ * created upon the Gerrit start. Note that Marshaller creation is a step that prepends the cache
+ * build therefore it is ensured that metrics object is always available for cache's Marshaller.
+ */
+class SerializationMetricsForCache {
+ protected static class Metrics {
+ final Timer0 deserializeLatency;
+ final Timer0 serializeLatency;
+
+ private Metrics(MetricMaker metricMaker, String cacheName) {
+ String sanitizedName = metricMaker.sanitizeMetricName(cacheName);
+
+ deserializeLatency =
+ metricMaker.newTimer(
+ "cache/chroniclemap/store_deserialize_latency_" + sanitizedName,
+ new Description(
+ String.format(
+ "The latency of deserializing entries from chronicle-map store for %s"
+ + " cache",
+ cacheName))
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+
+ serializeLatency =
+ metricMaker.newTimer(
+ "cache/chroniclemap/store_serialize_latency_" + sanitizedName,
+ new Description(
+ String.format(
+ "The latency of serializing entries to chronicle-map store for %s cache",
+ cacheName))
+ .setCumulative()
+ .setUnit(Units.NANOSECONDS));
+ }
+ }
+
+ private static final Map<String, Metrics> metricsCache = new HashMap<>();
+
+ protected final String name;
+ protected final transient Metrics metrics;
+
+ protected SerializationMetricsForCache(MetricMaker metricMaker, String name) {
+ this(createMetrics(metricMaker, name), name);
+ }
+
+ protected SerializationMetricsForCache(Metrics metrics, String name) {
+ this.metrics = Optional.ofNullable(metrics).orElseGet(() -> metricsCache.get(name));
+ this.name = name;
+ }
+
+ private static Metrics createMetrics(MetricMaker metricMaker, String name) {
+ Metrics metrics = new Metrics(metricMaker, name);
+ metricsCache.put(name, metrics);
+ return metrics;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java
index f26366f..13f12ec 100644
--- a/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java
+++ b/src/main/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshaller.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.googlesource.gerrit.modules.cache.chroniclemap;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
import java.nio.ByteBuffer;
import net.openhft.chronicle.bytes.Bytes;
@@ -20,74 +22,82 @@
import net.openhft.chronicle.hash.serialization.BytesReader;
import net.openhft.chronicle.hash.serialization.BytesWriter;
-public class TimedValueMarshaller<V>
+public class TimedValueMarshaller<V> extends SerializationMetricsForCache
implements BytesWriter<TimedValue<V>>,
BytesReader<TimedValue<V>>,
ReadResolvable<TimedValueMarshaller<V>> {
- private final String name;
private final CacheSerializer<V> cacheSerializer;
- TimedValueMarshaller(String name) {
- this.name = name;
+ TimedValueMarshaller(MetricMaker metricMaker, String name) {
+ super(metricMaker, name);
+ this.cacheSerializer = CacheSerializers.getValueSerializer(name);
+ }
+
+ private TimedValueMarshaller(Metrics metrics, String name) {
+ super(metrics, name);
this.cacheSerializer = CacheSerializers.getValueSerializer(name);
}
@Override
public TimedValueMarshaller<V> readResolve() {
- return new TimedValueMarshaller<>(name);
+ return new TimedValueMarshaller<>(metrics, name);
}
@SuppressWarnings("rawtypes")
@Override
public TimedValue<V> read(Bytes in, TimedValue<V> using) {
- long initialPosition = in.readPosition();
+ try (Timer0.Context timer = metrics.deserializeLatency.start()) {
+ long initialPosition = in.readPosition();
- // Deserialize the creation timestamp (first 8 bytes)
- byte[] serializedLong = new byte[Long.BYTES];
- in.read(serializedLong, 0, Long.BYTES);
- ByteBuffer buffer = ByteBuffer.wrap(serializedLong);
- long created = buffer.getLong(0);
- in.readPosition(initialPosition + Long.BYTES);
+ // Deserialize the creation timestamp (first 8 bytes)
+ byte[] serializedLong = new byte[Long.BYTES];
+ in.read(serializedLong, 0, Long.BYTES);
+ ByteBuffer buffer = ByteBuffer.wrap(serializedLong);
+ long created = buffer.getLong(0);
+ in.readPosition(initialPosition + Long.BYTES);
- // Deserialize the length of the serialized value (second 8 bytes)
- byte[] serializedInt = new byte[Integer.BYTES];
- in.read(serializedInt, 0, Integer.BYTES);
- ByteBuffer buffer2 = ByteBuffer.wrap(serializedInt);
- int vLength = buffer2.getInt(0);
- in.readPosition(initialPosition + Long.BYTES + Integer.BYTES);
+ // Deserialize the length of the serialized value (second 8 bytes)
+ byte[] serializedInt = new byte[Integer.BYTES];
+ in.read(serializedInt, 0, Integer.BYTES);
+ ByteBuffer buffer2 = ByteBuffer.wrap(serializedInt);
+ int vLength = buffer2.getInt(0);
+ in.readPosition(initialPosition + Long.BYTES + Integer.BYTES);
- // Deserialize object V (remaining bytes)
- byte[] serializedV = new byte[vLength];
- in.read(serializedV, 0, vLength);
- V v = cacheSerializer.deserialize(serializedV);
+ // Deserialize object V (remaining bytes)
+ byte[] serializedV = new byte[vLength];
+ in.read(serializedV, 0, vLength);
+ V v = cacheSerializer.deserialize(serializedV);
- using = new TimedValue<>(v, created);
+ using = new TimedValue<>(v, created);
- return using;
+ return using;
+ }
}
@SuppressWarnings("rawtypes")
@Override
public void write(Bytes out, TimedValue<V> toWrite) {
- byte[] serialized = cacheSerializer.serialize(toWrite.getValue());
+ try (Timer0.Context timer = metrics.serializeLatency.start()) {
+ byte[] serialized = cacheSerializer.serialize(toWrite.getValue());
- // Serialize as follows:
- // created | length of serialized V | serialized value V
- // 8 bytes | 4 bytes | serialized_length bytes
+ // Serialize as follows:
+ // created | length of serialized V | serialized value V
+ // 8 bytes | 4 bytes | serialized_length bytes
- int capacity = Long.BYTES + Integer.BYTES + serialized.length;
- ByteBuffer buffer = ByteBuffer.allocate(capacity);
+ int capacity = Long.BYTES + Integer.BYTES + serialized.length;
+ ByteBuffer buffer = ByteBuffer.allocate(capacity);
- long timestamp = toWrite.getCreated();
- buffer.putLong(0, timestamp);
+ long timestamp = toWrite.getCreated();
+ buffer.putLong(0, timestamp);
- buffer.position(Long.BYTES);
- buffer.putInt(serialized.length);
+ buffer.position(Long.BYTES);
+ buffer.putInt(serialized.length);
- buffer.position(Long.BYTES + Integer.BYTES);
- buffer.put(serialized);
+ buffer.position(Long.BYTES + Integer.BYTES);
+ buffer.put(serialized);
- out.write(buffer.array());
+ out.write(buffer.array());
+ }
}
}
diff --git a/src/main/resources/Documentation/metrics.md b/src/main/resources/Documentation/metrics.md
index 03b8918..e1ff273 100644
--- a/src/main/resources/Documentation/metrics.md
+++ b/src/main/resources/Documentation/metrics.md
@@ -38,6 +38,12 @@
* cache/chroniclemap/keys_index_persist_latency_<cache-name>
: The latency of persisting an index to a file.
+* cache/chroniclemap/store_serialize_latency_<cache-name>
+ : The latency of serializing entries in chronicle-map store
+
+* cache/chroniclemap/store_deserialize_latency_<cache-name>
+ : The latency of deserializing entries from chronicle-map store
+
* cache/chroniclemap/store_put_failures_<cache-name>
: The number of errors caught when inserting entries in chronicle-map store
@@ -45,4 +51,4 @@
: The number of errors caught when restore cache index from file operation was performed
* cache/chroniclemap/keys_index_persist_failures_<cache-name>
- : The number of errors caught when persist cache index to file operation was performed
\ No newline at end of file
+ : The number of errors caught when persist cache index to file operation was performed
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 d90eaff..72f23e5 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
@@ -557,7 +557,8 @@
}
private int valueSize(String value) {
- final TimedValueMarshaller<String> marshaller = new TimedValueMarshaller<>(testCacheName);
+ final TimedValueMarshaller<String> marshaller =
+ new TimedValueMarshaller<>(metricMaker, testCacheName);
Bytes<ByteBuffer> out = Bytes.elasticByteBuffer();
marshaller.write(out, new TimedValue<>(value));
diff --git a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshallerTest.java b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshallerTest.java
index b630e36..b4eb4ca 100644
--- a/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshallerTest.java
+++ b/src/test/java/com/googlesource/gerrit/modules/cache/chroniclemap/TimedValueMarshallerTest.java
@@ -15,6 +15,7 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.gerrit.acceptance.TestMetricMaker;
import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer;
import java.nio.ByteBuffer;
import net.openhft.chronicle.bytes.Bytes;
@@ -35,7 +36,8 @@
public void shouldSerializeAndDeserializeBack() {
ObjectId id = ObjectId.fromString("1234567890123456789012345678901234567890");
long timestamp = 1600329018L;
- TimedValueMarshaller<ObjectId> marshaller = new TimedValueMarshaller<>(TEST_CACHE_NAME);
+ TimedValueMarshaller<ObjectId> marshaller =
+ new TimedValueMarshaller<>(new TestMetricMaker(), TEST_CACHE_NAME);
final TimedValue<ObjectId> wrapped = new TimedValue<>(id, timestamp);