Remove extra log lines and add test cases

Signed-off-by: Sandesh Kumar <[email protected]>

server/src/main/java/org/opensearch/cluster/metadata/Metadata.java

@@ -55,7 +55,6 @@
 import org.opensearch.common.settings.Setting;
 import org.opensearch.common.settings.Setting.Property;
 import org.opensearch.common.settings.Settings;
-import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.xcontent.XContentHelper;
 import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.NamedObjectNotFoundException;
@@ -1429,7 +1428,6 @@ public Builder generateClusterUuidIfNeeded() {
         }
 
         public Metadata build() {
-            TimeValue buildStartTime = TimeValue.timeValueMillis(System.nanoTime());
             DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE);
             DataStreamMetadata previousDataStreamMetadata = (previousMetadata != null)
                 ? (DataStreamMetadata) this.previousMetadata.customs.get(DataStreamMetadata.TYPE)
@@ -1439,23 +1437,11 @@ public Metadata build() {
                 || (indices.equals(previousMetadata.indices) == false)
                 || (previousDataStreamMetadata != null && previousDataStreamMetadata.equals(dataStreamMetadata) == false)
                 || (dataStreamMetadata != null && dataStreamMetadata.equals(previousDataStreamMetadata) == false);
-            TimeValue recomputeEndTime = TimeValue.timeValueMillis(System.nanoTime());
-            logger.info(
-                "Recompute required: {}, time taken for comparing indices: {} ms",
-                recomputeRequired,
-                (recomputeEndTime.getNanos() - buildStartTime.getNanos()) / 1000000L
-            );
 
-            Metadata metadata = (recomputeRequired == false)
-                ? buildMetadataWithPreviousIndicesLookups()
-                : buildMetadataWithRecomputedIndicesLookups();
-            TimeValue endBuildTime = TimeValue.timeValueMillis(System.nanoTime());
-            // Logging for testing only - will remove in future iterations
-            logger.info("built metadata in {} ms", (endBuildTime.millis() - buildStartTime.millis()) / 1000000L);
-            return metadata;
+            return (recomputeRequired == false) ? buildMetadataWithPreviousIndicesLookups() : buildMetadataWithRecomputedIndicesLookups();
         }
 
-        private Metadata buildMetadataWithPreviousIndicesLookups() {
+        protected Metadata buildMetadataWithPreviousIndicesLookups() {
             return new Metadata(
                 clusterUUID,
                 clusterUUIDCommitted,
@@ -1473,11 +1459,11 @@ private Metadata buildMetadataWithPreviousIndicesLookups() {
                 Arrays.copyOf(previousMetadata.visibleOpenIndices, previousMetadata.visibleOpenIndices.length),
                 Arrays.copyOf(previousMetadata.allClosedIndices, previousMetadata.allClosedIndices.length),
                 Arrays.copyOf(previousMetadata.visibleClosedIndices, previousMetadata.visibleClosedIndices.length),
-                new TreeMap<>(previousMetadata.indicesLookup)
+                Collections.unmodifiableSortedMap(previousMetadata.indicesLookup)
             );
         }
 
-        private Metadata buildMetadataWithRecomputedIndicesLookups() {
+        protected Metadata buildMetadataWithRecomputedIndicesLookups() {
             // TODO: We should move these datastructures to IndexNameExpressionResolver, this will give the following benefits:
             // 1) The datastructures will be rebuilt only when needed. Now during serializing we rebuild these datastructures
             // while these datastructures aren't even used.
@@ -1564,15 +1550,7 @@ private Metadata buildMetadataWithRecomputedIndicesLookups() {
                 );
             }
 
-            TimeValue startTime = TimeValue.timeValueNanos(System.nanoTime());
             SortedMap<String, IndexAbstraction> indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup());
-            TimeValue endTime = TimeValue.timeValueNanos(System.nanoTime());
-            // Logging for testing only - will remove in future iterations
-            logger.debug(
-                "rebuilt indicesLookupMap in {} ms, {} entries",
-                (endTime.nanos() - startTime.nanos()) / 1000000L,
-                indicesLookup.size()
-            );
 
             validateDataStreams(indicesLookup, (DataStreamMetadata) customs.get(DataStreamMetadata.TYPE));
 

server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java

@@ -32,6 +32,7 @@
 
 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;
@@ -58,6 +59,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -1435,4 +1437,50 @@ private static class CreateIndexResult {
             this.metadata = metadata;
         }
     }
+
+    public void testMetadataBuild() {
+        Metadata previousMetadata = randomMetadata();
+        Metadata.Builder spyBuilder = Mockito.spy(Metadata.builder());
+
+        // 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();
+
+        // 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(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())
+            .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);
+
+        // Adding new data-stream - indices lookups should get re-computed
+        spyBuilder = Mockito.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();
+    }
 }