[Optimization] Cluster State Update Optimization

CHANGELOG.md

@@ -94,6 +94,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Changed
 - Replace jboss-annotations-api_1.2_spec with jakarta.annotation-api ([#7836](https://github.com/opensearch-project/OpenSearch/pull/7836))
 - Add min, max, average and thread info to resource stats in tasks API ([#7673](https://github.com/opensearch-project/OpenSearch/pull/7673))
+- Optimized Metadata build() to skip redundant computations as part of ClusterState build ([#7853](https://github.com/opensearch-project/OpenSearch/pull/7853))
 
 ### Deprecated
 

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

@@ -55,6 +55,7 @@
 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;
@@ -1119,14 +1120,25 @@ public static class Builder {
         private DiffableStringMap hashesOfConsistentSettings = new DiffableStringMap(Collections.emptyMap());
 
         private final Map<String, IndexMetadata> indices;
+        private final Map<String, IndexMetadata> indicesPreviousState;
         private final Map<String, IndexTemplateMetadata> templates;
         private final Map<String, Custom> customs;
+        private final Map<String, Custom> customsPreviousState;
+        public SortedMap<String, IndexAbstraction> indicesLookup = new TreeMap<>();
+        private String[] allIndices = new String[0];
+        private String[] visibleIndices = new String[0];
+        private String[] allOpenIndices = new String[0];
+        private String[] visibleOpenIndices = new String[0];
+        private String[] allClosedIndices = new String[0];
+        private String[] visibleClosedIndices = new String[0];
 
         public Builder() {
             clusterUUID = UNKNOWN_CLUSTER_UUID;
             indices = new HashMap<>();
+            indicesPreviousState = new HashMap<>();
             templates = new HashMap<>();
             customs = new HashMap<>();
+            customsPreviousState = new HashMap<>();
             indexGraveyard(IndexGraveyard.builder().build()); // create new empty index graveyard to initialize
         }
 
@@ -1139,8 +1151,17 @@ public Builder(Metadata metadata) {
             this.hashesOfConsistentSettings = metadata.hashesOfConsistentSettings;
             this.version = metadata.version;
             this.indices = new HashMap<>(metadata.indices);
+            this.indicesPreviousState = new HashMap<>(metadata.indices); // required for comparing with updated indices
             this.templates = new HashMap<>(metadata.templates);
             this.customs = new HashMap<>(metadata.customs);
+            this.customsPreviousState = new HashMap<>(metadata.customs);
+            this.indicesLookup = new TreeMap<>(metadata.indicesLookup);
+            this.allIndices = Arrays.copyOf(metadata.allIndices, metadata.allIndices.length);
+            this.visibleIndices = Arrays.copyOf(metadata.visibleIndices, metadata.visibleIndices.length);
+            this.allOpenIndices = Arrays.copyOf(metadata.allOpenIndices, metadata.allOpenIndices.length);
+            this.visibleOpenIndices = Arrays.copyOf(metadata.visibleOpenIndices, metadata.visibleOpenIndices.length);
+            this.allClosedIndices = Arrays.copyOf(metadata.allClosedIndices, metadata.allClosedIndices.length);
+            this.visibleClosedIndices = Arrays.copyOf(metadata.visibleClosedIndices, metadata.visibleClosedIndices.length);
         }
 
         public Builder put(IndexMetadata.Builder indexMetadataBuilder) {
@@ -1425,6 +1446,45 @@ public Builder generateClusterUuidIfNeeded() {
         }
 
         public Metadata build() {
+            TimeValue buildStartTime = TimeValue.timeValueMillis(System.nanoTime());
+            boolean recomputeRequired = indices.equals(indicesPreviousState) == false || customs.equals(customsPreviousState) == false;
+            TimeValue recomputeEndTime = TimeValue.timeValueMillis(System.nanoTime());
+            logger.info(
+                "Recompute required: {}, time taken for comparing indices: {} ms",
+                recomputeRequired,
+                (recomputeEndTime.getNanos() - buildStartTime.getNanos()) / 1000000L
+            );
+            // Will simplify this later to omit this recomputeRequired variable entirely - it's only used for testing for now
+            if (recomputeRequired) {
+                buildMetadataIndicesLookups();
+            }
+
+            Metadata metadata = new Metadata(
+                clusterUUID,
+                clusterUUIDCommitted,
+                version,
+                coordinationMetadata,
+                transientSettings,
+                persistentSettings,
+                hashesOfConsistentSettings,
+                indices,
+                templates,
+                customs,
+                allIndices,
+                visibleIndices,
+                allOpenIndices,
+                visibleOpenIndices,
+                allClosedIndices,
+                visibleClosedIndices,
+                indicesLookup
+            );
+            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;
+        }
+
+        private void buildMetadataIndicesLookups() {
             // 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.
@@ -1511,40 +1571,28 @@ public Metadata build() {
                 );
             }
 
-            SortedMap<String, IndexAbstraction> indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup());
+            TimeValue startTime = TimeValue.timeValueNanos(System.nanoTime());
+            indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup());
+            TimeValue endTime = TimeValue.timeValueNanos(System.nanoTime());
+            // Logging for testing only - will remove in future iterations
+            logger.info(
+                "rebuilt indicesLookupMap in {} ms, {} entries",
+                (endTime.nanos() - startTime.nanos()) / 1000000L,
+                indicesLookup != null ? indicesLookup.size() : 0
+            );
 
             validateDataStreams(indicesLookup, (DataStreamMetadata) customs.get(DataStreamMetadata.TYPE));
 
             // build all concrete indices arrays:
             // TODO: I think we can remove these arrays. it isn't worth the effort, for operations on all indices.
             // When doing an operation across all indices, most of the time is spent on actually going to all shards and
             // do the required operations, the bottleneck isn't resolving expressions into concrete indices.
-            String[] allIndicesArray = allIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] visibleIndicesArray = visibleIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] allOpenIndicesArray = allOpenIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] visibleOpenIndicesArray = visibleOpenIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] allClosedIndicesArray = allClosedIndices.toArray(Strings.EMPTY_ARRAY);
-            String[] visibleClosedIndicesArray = visibleClosedIndices.toArray(Strings.EMPTY_ARRAY);
-
-            return new Metadata(
-                clusterUUID,
-                clusterUUIDCommitted,
-                version,
-                coordinationMetadata,
-                transientSettings,
-                persistentSettings,
-                hashesOfConsistentSettings,
-                indices,
-                templates,
-                customs,
-                allIndicesArray,
-                visibleIndicesArray,
-                allOpenIndicesArray,
-                visibleOpenIndicesArray,
-                allClosedIndicesArray,
-                visibleClosedIndicesArray,
-                indicesLookup
-            );
+            this.allIndices = allIndices.toArray(new String[allIndices.size()]);
+            this.allOpenIndices = allOpenIndices.toArray(new String[allOpenIndices.size()]);
+            this.allClosedIndices = allClosedIndices.toArray(new String[allClosedIndices.size()]);
+            this.visibleIndices = visibleIndices.toArray(new String[visibleIndices.size()]);
+            this.visibleOpenIndices = visibleOpenIndices.toArray(new String[visibleOpenIndices.size()]);
+            this.visibleClosedIndices = visibleClosedIndices.toArray(new String[visibleClosedIndices.size()]);
         }
 
         private SortedMap<String, IndexAbstraction> buildIndicesLookup() {

server/src/main/java/org/opensearch/cluster/service/MasterService.java

@@ -292,10 +292,13 @@ private void runTasks(TaskInputs taskInputs) {
         }
 
         final long computationStartTime = threadPool.relativeTimeInMillis();
+        TimeValue startTime = TimeValue.timeValueMillis(System.currentTimeMillis());
         final TaskOutputs taskOutputs = calculateTaskOutputs(taskInputs, previousClusterState);
         taskOutputs.notifyFailedTasks();
         final TimeValue computationTime = getTimeSince(computationStartTime);
-        logExecutionTime(computationTime, "compute cluster state update", summary);
+        TimeValue endTime = TimeValue.timeValueMillis(System.currentTimeMillis());
+        // Temporarily logging time diff from System time since threadpool time is not precise for less than 200ms
+        logExecutionTime(TimeValue.timeValueMillis(endTime.millis() - startTime.millis()), "compute cluster state update", summary);
 
         if (taskOutputs.clusterStateUnchanged()) {
             final long notificationStartTime = threadPool.relativeTimeInMillis();