From 7d64109e647c5a321cf989006e9e99b873abcda9 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 15 Dec 2021 12:31:57 +0100 Subject: Avoid creating many sets and lists containing the same list of consumers. Use a dictionary to keep track of the unique ones. --- .../metricsproxy/metric/model/MetricsPacket.java | 8 ++-- .../metric/model/json/YamasJsonModel.java | 23 ----------- .../metric/model/json/YamasJsonUtil.java | 46 +++++++++++----------- .../metricsproxy/metric/ExternalMetricsTest.java | 9 +++-- .../metric/model/json/YamasJsonModelTest.java | 5 ++- 5 files changed, 36 insertions(+), 55 deletions(-) (limited to 'metrics-proxy') diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java index b08c3f4d668..02ccd2c988c 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/MetricsPacket.java @@ -36,7 +36,7 @@ public class MetricsPacket { public final ServiceId service; private final Map metrics; private final Map dimensions; - private final List consumers; + private final Set consumers; private MetricsPacket(int statusCode, String statusMessage, long timestamp, ServiceId service, Map metrics, Map dimensions, Set consumers ) { @@ -46,7 +46,7 @@ public class MetricsPacket { this.service = service; this.metrics = metrics; this.dimensions = dimensions; - this.consumers = new ArrayList<>(consumers); + this.consumers = Set.copyOf(consumers); } public Map metrics() { @@ -57,8 +57,8 @@ public class MetricsPacket { return unmodifiableMap(dimensions); } - public List consumers() { - return unmodifiableList(consumers); + public Set consumers() { + return consumers; } @Override diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java index e86ab2ab41a..d2be7be25d1 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModel.java @@ -59,29 +59,6 @@ public class YamasJsonModel { newMetrics.forEach(metric -> metrics.put(metric.getName().id, metric.getValue().doubleValue())); } - /** - * Convenience method to add targets to the routing object - * - * @param names Namespaces E.g "Vespa" - */ - public void addRouting(Set names) { - // Setup routing structure if not already existing - if (routing == null) { - routing = new HashMap<>(); - } - - if (! routing.containsKey("yamas")) { - routing.put("yamas", new YamasJsonModel.YamasJsonNamespace()); - } - YamasJsonModel.YamasJsonNamespace namespace = routing.get("yamas"); - - if (namespace.namespaces == null) { - namespace.namespaces = new ArrayList<>(); - } - - namespace.namespaces.addAll(names.stream().map(consumer -> consumer.id).collect(Collectors.toList())); - } - /** * Convenience method to add dimensions */ diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java index 19bcdbfdb74..602879aa0b6 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonUtil.java @@ -7,13 +7,14 @@ import ai.vespa.metricsproxy.metric.model.ServiceId; import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableMap; +import com.yahoo.concurrent.CopyOnWriteHashMap; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -30,6 +31,7 @@ public class YamasJsonUtil { static final String YAMAS_ROUTING = "yamas"; + private static final Map, Map> globalNameSpaces = new CopyOnWriteHashMap<>(); public static MetricsPacket.Builder toMetricsPacketBuilder(YamasJsonModel jsonModel) { if (jsonModel.application == null) throw new IllegalArgumentException("Service id cannot be null"); @@ -79,12 +81,12 @@ public class YamasJsonUtil { } } - private static YamasJsonModel getStatusYamasModel(String statusMessage, int statusCode, Collection consumers) { + private static YamasJsonModel getStatusYamasModel(String statusMessage, int statusCode, Set consumers) { YamasJsonModel model = new YamasJsonModel(); model.status_code = statusCode; model.status_msg = statusMessage; model.application = "yms_check_vespa"; - model.routing = ImmutableMap.of(YAMAS_ROUTING, toYamasJsonNamespaces(consumers)); + model.routing = computeIfAbsent(consumers); return model; } @@ -99,38 +101,38 @@ public class YamasJsonUtil { model.application = packet.service.id; model.timestamp = (packet.timestamp == 0L) ? null : packet.timestamp; - if (packet.metrics().isEmpty()) model.metrics = null; - else { - model.metrics = packet.metrics().entrySet().stream().collect( + model.metrics = (packet.metrics().isEmpty()) + ? null + : packet.metrics().entrySet().stream().collect( toLinkedMap(id2metric -> id2metric.getKey().id, id2metric -> id2metric.getValue().doubleValue())); - } - if (packet.dimensions().isEmpty()) model.dimensions = null; - else { - model.dimensions = packet.dimensions().entrySet() + model.dimensions = (packet.dimensions().isEmpty()) + ? null + : packet.dimensions().entrySet() .stream() .filter(entry -> entry.getKey() != null && entry.getValue() != null) - .collect(toLinkedMap( - id2dim -> id2dim.getKey().id, - Map.Entry::getValue) - ); - } + .collect(toLinkedMap(id2dim -> id2dim.getKey().id, Map.Entry::getValue)); - YamasJsonModel.YamasJsonNamespace namespaces = toYamasJsonNamespaces(packet.consumers()); - if (namespaces.namespaces.isEmpty()) model.routing = null; - else model.routing = ImmutableMap.of(YAMAS_ROUTING, namespaces); + model.routing = computeIfAbsent(packet.consumers()); return model; } - private static YamasJsonModel.YamasJsonNamespace toYamasJsonNamespaces(Collection consumers) { - YamasJsonModel.YamasJsonNamespace namespaces = new YamasJsonModel.YamasJsonNamespace(); - namespaces.namespaces = consumers.stream() + private static Map computeIfAbsent(Set consumers) { + return globalNameSpaces.computeIfAbsent(consumers, YamasJsonUtil::createYamasJson); + } + + private static Map createYamasJson(Set consumers) { + List namespaces = consumers.stream() .filter(consumerId -> consumerId != defaultMetricsConsumerId) .map(consumer -> consumer.id) .collect(Collectors.toList()); - return namespaces; + if (namespaces.isEmpty()) return null; + + YamasJsonModel.YamasJsonNamespace yamasJsonNamespace = new YamasJsonModel.YamasJsonNamespace(); + yamasJsonNamespace.namespaces = namespaces; + return Map.of(YAMAS_ROUTING, yamasJsonNamespace); } } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java index 002e6334141..581367878ca 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/ExternalMetricsTest.java @@ -10,11 +10,12 @@ import com.google.common.collect.ImmutableList; import org.junit.Test; import java.util.List; +import java.util.Set; -import static ai.vespa.metricsproxy.metric.ExternalMetrics.VESPA_NODE_SERVICE_ID; import static ai.vespa.metricsproxy.metric.model.ConsumerId.toConsumerId; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * @author gjoranv @@ -64,10 +65,10 @@ public class ExternalMetricsTest { List packets = externalMetrics.getMetrics(); assertEquals(1, packets.size()); - List consumerIds = packets.get(0).build().consumers(); + Set consumerIds = packets.get(0).build().consumers(); assertEquals(2, consumerIds.size()); - assertEquals(CUSTOM_CONSUMER_1, consumerIds.get(0)); - assertEquals(CUSTOM_CONSUMER_2, consumerIds.get(1)); + assertTrue(consumerIds.contains(CUSTOM_CONSUMER_1)); + assertTrue(consumerIds.contains(CUSTOM_CONSUMER_2)); } } diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java index 9b889085a87..540445fba5b 100644 --- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/metric/model/json/YamasJsonModelTest.java @@ -16,6 +16,7 @@ import static ai.vespa.metricsproxy.metric.model.MetricId.toMetricId; import static ai.vespa.metricsproxy.metric.model.ServiceId.toServiceId; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; /** * Tests for YamasJsonModel and YamasArrayJsonModel @@ -43,7 +44,7 @@ public class YamasJsonModelTest { // Do some sanity checking assertEquals("vespa.searchnode", jsonModel.application); - assertEquals("Vespa", jsonModel.routing.get("yamas").namespaces.get(0)); + assertTrue(jsonModel.routing.get("yamas").namespaces.contains("Vespa")); assertEquals(5.555555555E9, jsonModel.metrics.get("memory_rss"), 0.1d); //Not using custom double renderer // Serialize and verify @@ -60,7 +61,7 @@ public class YamasJsonModelTest { // Do some sanity checking assertEquals(toServiceId("vespa.searchnode"), metricsPacket.service); - assertEquals(toConsumerId("Vespa"), metricsPacket.consumers().get(0)); + assertTrue(metricsPacket.consumers().contains(toConsumerId("Vespa"))); assertEquals(5.555555555E9, metricsPacket.metrics().get(toMetricId("memory_rss")).doubleValue(), 0.1d); //Not using custom double rendrer // Serialize and verify -- cgit v1.2.3