From 38d754e6e161309f9d9c4e5e8883d814d516eda2 Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Thu, 7 Nov 2024 16:06:24 +0100 Subject: [PATCH] Deduplicate the list of names when deserializing InternalTopMetrics (#116298) use deduplication infrastructure to deduplicate the names of metrics in InternalTopMetrics. --- .../common/lucene/LuceneTests.java | 20 +++++++++++++++---- .../org/elasticsearch/test/ESTestCase.java | 20 +++++++++++++++---- .../topmetrics/InternalTopMetrics.java | 8 +++++++- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/common/lucene/LuceneTests.java b/server/src/test/java/org/elasticsearch/common/lucene/LuceneTests.java index 9300aa992b687..d5fb33c9ec671 100644 --- a/server/src/test/java/org/elasticsearch/common/lucene/LuceneTests.java +++ b/server/src/test/java/org/elasticsearch/common/lucene/LuceneTests.java @@ -50,7 +50,10 @@ import org.apache.lucene.tests.store.MockDirectoryWrapper; import org.apache.lucene.util.Bits; import org.apache.lucene.util.BytesRef; -import org.elasticsearch.common.io.stream.NamedWriteableRegistry; +import org.elasticsearch.TransportVersion; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.core.IOUtils; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.fielddata.IndexFieldData; @@ -74,7 +77,6 @@ import static org.hamcrest.Matchers.equalTo; public class LuceneTests extends ESTestCase { - private static final NamedWriteableRegistry EMPTY_REGISTRY = new NamedWriteableRegistry(Collections.emptyList()); public void testCleanIndex() throws IOException { MockDirectoryWrapper dir = newMockDirectory(); @@ -551,7 +553,6 @@ public void testSortFieldSerialization() throws IOException { Tuple sortFieldTuple = randomSortField(); SortField deserialized = copyInstance( sortFieldTuple.v1(), - EMPTY_REGISTRY, Lucene::writeSortField, Lucene::readSortField, TransportVersionUtils.randomVersion(random()) @@ -563,7 +564,6 @@ public void testSortValueSerialization() throws IOException { Object sortValue = randomSortValue(); Object deserialized = copyInstance( sortValue, - EMPTY_REGISTRY, Lucene::writeSortValue, Lucene::readSortValue, TransportVersionUtils.randomVersion(random()) @@ -571,6 +571,18 @@ public void testSortValueSerialization() throws IOException { assertEquals(sortValue, deserialized); } + private static T copyInstance(T original, Writeable.Writer writer, Writeable.Reader reader, TransportVersion version) + throws IOException { + try (BytesStreamOutput output = new BytesStreamOutput()) { + output.setTransportVersion(version); + writer.write(output, original); + try (StreamInput in = output.bytes().streamInput()) { + in.setTransportVersion(version); + return reader.read(in); + } + } + } + public static Object randomSortValue() { return switch (randomIntBetween(0, 9)) { case 0 -> null; diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 5bfcd54e963b3..1edc800956a67 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -65,6 +65,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.bytes.CompositeBytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.DelayableWriteable; import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -1849,7 +1850,7 @@ public static C copyNamedWriteable( ); } - protected static T copyInstance( + protected static T copyInstance( T original, NamedWriteableRegistry namedWriteableRegistry, Writeable.Writer writer, @@ -1859,9 +1860,20 @@ protected static T copyInstance( try (BytesStreamOutput output = new BytesStreamOutput()) { output.setTransportVersion(version); writer.write(output, original); - try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) { - in.setTransportVersion(version); - return reader.read(in); + if (randomBoolean()) { + try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) { + in.setTransportVersion(version); + return reader.read(in); + } + } else { + BytesReference bytesReference = output.copyBytes(); + output.reset(); + output.writeBytesReference(bytesReference); + try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) { + in.setTransportVersion(version); + DelayableWriteable delayableWriteable = DelayableWriteable.delayed(reader, in); + return delayableWriteable.expand(); + } } } } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java index f5c8a14c314cc..ba389a6cd9d7c 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/InternalTopMetrics.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.analytics.topmetrics; import org.apache.lucene.util.PriorityQueue; +import org.elasticsearch.common.io.stream.DelayableWriteable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -68,7 +69,12 @@ static InternalTopMetrics buildEmptyAggregation(String name, List metric public InternalTopMetrics(StreamInput in) throws IOException { super(in); sortOrder = SortOrder.readFromStream(in); - metricNames = in.readStringCollectionAsList(); + final List metricNames = in.readStringCollectionAsList(); + if (in instanceof DelayableWriteable.Deduplicator bo) { + this.metricNames = bo.deduplicate(metricNames); + } else { + this.metricNames = metricNames; + } size = in.readVInt(); topMetrics = in.readCollectionAsList(TopMetric::new); }