diff --git a/CHANGELOG.md b/CHANGELOG.md index b73b9a028ac0b..8219d6d1d9164 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,7 +141,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add self-organizing hash table to improve the performance of bucket aggregations ([#7652](https://github.com/opensearch-project/OpenSearch/pull/7652)) - Check UTF16 string size before converting to String to avoid OOME ([#7963](https://github.com/opensearch-project/OpenSearch/pull/7963)) - Move ZSTD compression codecs out of the sandbox ([#7908](https://github.com/opensearch-project/OpenSearch/pull/7908)) -- Optimized Metadata build() to skip redundant computations as part of ClusterState build ([#7853](https://github.com/opensearch-project/OpenSearch/pull/7853)) +- Optimize Metadata build() to skip redundant computations as part of ClusterState build ([#7853](https://github.com/opensearch-project/OpenSearch/pull/7853)) ### Deprecated diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java index da2a779903f7c..fed06e7a21ce9 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java @@ -32,7 +32,6 @@ package org.opensearch.cluster.metadata; -import org.mockito.Mockito; import org.opensearch.Version; import org.opensearch.action.admin.indices.alias.get.GetAliasesRequest; import org.opensearch.cluster.ClusterModule; @@ -78,6 +77,10 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class MetadataTests extends OpenSearchTestCase { @@ -1367,52 +1370,59 @@ public void testValidateDataStreamsForNullDataStreamMetadata() { } public void testMetadataBuildInvocations() { - Metadata previousMetadata = randomMetadata(); - Metadata.Builder spyBuilder = Mockito.spy(Metadata.builder()); + final Metadata previousMetadata = randomMetadata(); + Metadata builtMetadata; + Metadata.Builder spyBuilder; // previous Metadata state was not provided to Builder during assignment - indices lookups should get re-computed - spyBuilder.build(); - Mockito.verify(spyBuilder, Mockito.times(1)).buildMetadataWithRecomputedIndicesLookups(); - Mockito.verify(spyBuilder, Mockito.times(0)).buildMetadataWithPreviousIndicesLookups(); + spyBuilder = spy(Metadata.builder()); + builtMetadata = spyBuilder.build(); + verify(spyBuilder, times(1)).buildMetadataWithRecomputedIndicesLookups(); + verify(spyBuilder, times(0)).buildMetadataWithPreviousIndicesLookups(); + compareMetadata(Metadata.EMPTY_METADATA, builtMetadata, true, true, false); // no changes in builder method after initialization from previous Metadata - indices lookups should not be re-computed - spyBuilder = Mockito.spy(Metadata.builder(previousMetadata)); - spyBuilder.build(); - Mockito.verify(spyBuilder, Mockito.times(0)).buildMetadataWithRecomputedIndicesLookups(); - Mockito.verify(spyBuilder, Mockito.times(1)).buildMetadataWithPreviousIndicesLookups(); - Mockito.reset(spyBuilder); - - // Adding new index - all indices lookups should get re-computed - spyBuilder = Mockito.spy(Metadata.builder(previousMetadata)); - String index = "new-index"; - spyBuilder.indices( + spyBuilder = spy(Metadata.builder(previousMetadata)); + builtMetadata = spyBuilder.version(previousMetadata.version() + 1).build(); + verify(spyBuilder, times(0)).buildMetadataWithRecomputedIndicesLookups(); + verify(spyBuilder, times(1)).buildMetadataWithPreviousIndicesLookups(); + compareMetadata(previousMetadata, builtMetadata, true, true, true); + reset(spyBuilder); + + // adding new index - all indices lookups should get re-computed + spyBuilder = spy(Metadata.builder(previousMetadata)); + String index = "new_index_" + randomAlphaOfLength(3); + builtMetadata = spyBuilder.indices( Collections.singletonMap( index, IndexMetadata.builder(index).settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(1).build() ) ).build(); - Mockito.verify(spyBuilder, Mockito.times(1)).buildMetadataWithRecomputedIndicesLookups(); - Mockito.verify(spyBuilder, Mockito.times(0)).buildMetadataWithPreviousIndicesLookups(); - Mockito.reset(spyBuilder); - - // Adding new templates - indices lookups should not get recomputed - spyBuilder = Mockito.spy(Metadata.builder(previousMetadata)); - spyBuilder.put("component_template_new_" + randomAlphaOfLength(3), ComponentTemplateTests.randomInstance()) + verify(spyBuilder, times(1)).buildMetadataWithRecomputedIndicesLookups(); + verify(spyBuilder, times(0)).buildMetadataWithPreviousIndicesLookups(); + compareMetadata(previousMetadata, builtMetadata, false, true, false); + reset(spyBuilder); + + // adding new templates - indices lookups should not get recomputed + spyBuilder = spy(Metadata.builder(previousMetadata)); + builtMetadata = spyBuilder.put("component_template_new_" + randomAlphaOfLength(3), ComponentTemplateTests.randomInstance()) .put("index_template_v2_new_" + randomAlphaOfLength(3), ComposableIndexTemplateTests.randomInstance()) .build(); - Mockito.verify(spyBuilder, Mockito.times(0)).buildMetadataWithRecomputedIndicesLookups(); - Mockito.verify(spyBuilder, Mockito.times(1)).buildMetadataWithPreviousIndicesLookups(); - Mockito.reset(spyBuilder); + verify(spyBuilder, times(0)).buildMetadataWithRecomputedIndicesLookups(); + verify(spyBuilder, times(1)).buildMetadataWithPreviousIndicesLookups(); + compareMetadata(previousMetadata, builtMetadata, true, false, false); + reset(spyBuilder); - // Adding new data-stream - indices lookups should get re-computed - spyBuilder = Mockito.spy(Metadata.builder(previousMetadata)); + // adding new data stream - indices lookups should get re-computed + spyBuilder = spy(Metadata.builder(previousMetadata)); DataStream dataStream = DataStreamTests.randomInstance(); for (Index backingIndex : dataStream.getIndices()) { spyBuilder.put(DataStreamTestHelper.getIndexMetadataBuilderForIndex(backingIndex)); } - spyBuilder.put(dataStream).build(); - Mockito.verify(spyBuilder, Mockito.times(1)).buildMetadataWithRecomputedIndicesLookups(); - Mockito.verify(spyBuilder, Mockito.times(0)).buildMetadataWithPreviousIndicesLookups(); + builtMetadata = spyBuilder.put(dataStream).version(previousMetadata.version() + 1).build(); + verify(spyBuilder, times(1)).buildMetadataWithRecomputedIndicesLookups(); + verify(spyBuilder, times(0)).buildMetadataWithPreviousIndicesLookups(); + compareMetadata(previousMetadata, builtMetadata, false, true, true); } public static Metadata randomMetadata() { @@ -1486,4 +1496,42 @@ private static class CreateIndexResult { this.metadata = metadata; } } + + private static void compareMetadata( + final Metadata previousMetadata, + final Metadata newMetadata, + final boolean compareIndicesLookups, + final boolean compareTemplates, + final boolean checkVersionIncrement + ) { + assertEquals(previousMetadata.clusterUUID(), newMetadata.clusterUUID()); + assertEquals(previousMetadata.clusterUUIDCommitted(), newMetadata.clusterUUIDCommitted()); + assertEquals(previousMetadata.coordinationMetadata(), newMetadata.coordinationMetadata()); + assertEquals(previousMetadata.settings(), newMetadata.settings()); + assertEquals(previousMetadata.transientSettings(), newMetadata.transientSettings()); + assertEquals(previousMetadata.persistentSettings(), newMetadata.persistentSettings()); + assertEquals(previousMetadata.hashesOfConsistentSettings(), newMetadata.hashesOfConsistentSettings()); + + if (compareIndicesLookups == true) { + assertEquals(previousMetadata.indices(), newMetadata.indices()); + assertEquals(previousMetadata.getConcreteAllIndices(), newMetadata.getConcreteAllIndices()); + assertEquals(previousMetadata.getConcreteAllClosedIndices(), newMetadata.getConcreteAllClosedIndices()); + assertEquals(previousMetadata.getConcreteAllOpenIndices(), newMetadata.getConcreteAllOpenIndices()); + assertEquals(previousMetadata.getConcreteVisibleIndices(), newMetadata.getConcreteVisibleIndices()); + assertEquals(previousMetadata.getConcreteVisibleClosedIndices(), newMetadata.getConcreteVisibleClosedIndices()); + assertEquals(previousMetadata.getConcreteVisibleOpenIndices(), newMetadata.getConcreteVisibleOpenIndices()); + assertEquals(previousMetadata.getIndicesLookup(), newMetadata.getIndicesLookup()); + assertEquals(previousMetadata.getCustoms(), newMetadata.getCustoms()); + } + + if (compareTemplates == true) { + assertEquals(previousMetadata.templates(), newMetadata.templates()); + assertEquals(previousMetadata.templatesV2(), newMetadata.templatesV2()); + assertEquals(previousMetadata.componentTemplates(), newMetadata.componentTemplates()); + } + + if (checkVersionIncrement == true) { + assertEquals(previousMetadata.version() + 1, newMetadata.version()); + } + } }