Deduplicate the list of names when deserializing InternalTopMetrics #116298
Conversation
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
| @@ -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()); | |||
There was a problem hiding this comment.
Was this just unused and you're cleaning it up? It's unclear to me what this removal has to do with the main change. No worries if you're just cleaning stuff up, just want to make sure I'm not missing something.
There was a problem hiding this comment.
This was used for serialization / deserialization using the generic method ESTestCase#copyInstance which required a registry. We cannot use that method anymore because it requires to implement Writable now.
| try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(), namedWriteableRegistry)) { | ||
| in.setTransportVersion(version); | ||
| return reader.read(in); | ||
| if (randomBoolean()) { |
There was a problem hiding this comment.
Nit - Is it worth adding some logging here so we can figure out what branch we're in if a test fails?
There was a problem hiding this comment.
I normally rely on the seed to understand why it has failed.
| Lucene::writeSortValue, | ||
| Lucene::readSortValue, | ||
| TransportVersionUtils.randomVersion(random()) | ||
| ); | ||
| assertEquals(sortValue, deserialized); | ||
| } | ||
|
|
||
| private static <T> T copyInstance(T original, Writeable.Writer<T> writer, Writeable.Reader<T> reader, TransportVersion version) |
There was a problem hiding this comment.
It's unclear to me what this method is for. It's private, and I don't see where it's being called? I'm probably just missing something...
There was a problem hiding this comment.
This method is to test the serialization / deserialization of SortValues. We cannot use the generic method ESTestCase#copyInstance because Sort values do not implement Writable. Instead we are using here: Lucene::writeSortValue and Lucene::readSortValue.
iverase
added
the
auto-backport
…lastic#116298) use deduplication infrastructure to deduplicate the names of metrics in InternalTopMetrics.
💚 Backport successful
|
…lastic#116298) use deduplication infrastructure to deduplicate the names of metrics in InternalTopMetrics.
…116298) use deduplication infrastructure to deduplicate the names of metrics in InternalTopMetrics.
…lastic#116298) use deduplication infrastructure to deduplicate the names of metrics in InternalTopMetrics.
In #112707 we added infrastructure to deduplicate objects where deserailizing internal aggregations. Let' use it to deduplicate the names of metrics in InternalTopMetrics.