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);