From 877e72233a4192847b0629d01b89b4261d565427 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 7 Oct 2021 15:02:45 +0200 Subject: [PATCH] Allow indices lookup to be built lazily (#78745) Overloaded the Metadata.Builder.build() method by adding a parameter that controls when to build indices lookup. By default the indices lookup is always build. When cluster state updates are batched in ilm, then built the indices lookup lazily. Because during batching only the final cluster state is used, then for the many intermediate cluster states we avoids building the indices lookup, which in a cluster with many indices can improve the performance of publishing cluster states significantly. Relates to #77466 --- .../cluster/metadata/Metadata.java | 52 +++++++++++++------ .../core/ilm/CopyExecutionStateStep.java | 2 +- .../xpack/core/ilm/CopySettingsStep.java | 2 +- .../core/ilm/GenerateSnapshotNameStep.java | 3 +- .../core/ilm/GenerateUniqueIndexNameStep.java | 2 +- .../core/ilm/InitializePolicyContextStep.java | 2 +- .../xpack/core/ilm/PhaseCacheManagement.java | 4 +- .../ReplaceDataStreamBackingIndexStep.java | 2 +- .../ilm/UpdateRolloverLifecycleDateStep.java | 2 +- .../xpack/ilm/IndexLifecycleRunner.java | 2 + .../xpack/ilm/IndexLifecycleTransition.java | 3 +- 11 files changed, 50 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java index b92a2fbb4538c..da7eb6d0f69b4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java @@ -189,7 +189,7 @@ public interface NonRestorableCustom extends Custom { private final String[] allClosedIndices; private final String[] visibleClosedIndices; - private final SortedMap indicesLookup; + private SortedMap indicesLookup; Metadata(String clusterUUID, boolean clusterUUIDCommitted, long version, CoordinationMetadata coordinationMetadata, Settings transientSettings, Settings persistentSettings, DiffableStringMap hashesOfConsistentSettings, @@ -304,6 +304,11 @@ public boolean equalsAliases(Metadata other) { } public SortedMap getIndicesLookup() { + if (indicesLookup != null) { + return indicesLookup; + } + DataStreamMetadata dataStreamMetadata = custom(DataStreamMetadata.TYPE); + indicesLookup = Collections.unmodifiableSortedMap(Builder.buildIndicesLookup(dataStreamMetadata, indices)); return indicesLookup; } @@ -1430,7 +1435,22 @@ public Builder generateClusterUuidIfNeeded() { return this; } + /** + * @return a new Metadata instance + */ public Metadata build() { + return build(true); + } + + /** + * @param builtIndicesLookupEagerly Controls whether indices lookup should be build as part of the execution of this method + * or after when needed. Almost all of the time indices lookup should be built eagerly, however + * in certain cases when Metdata instances are build that are not published and + * many indices have been defined then it makes sense to skip building indices lookup. + * + * @return a new Metadata instance + */ + public Metadata build(boolean builtIndicesLookupEagerly) { // 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. @@ -1514,9 +1534,15 @@ public Metadata build() { "were found [" + Strings.collectionToCommaDelimitedString(duplicates) + "]"); } - SortedMap indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup()); + ImmutableOpenMap indices = this.indices.build(); + + SortedMap indicesLookup; + if (builtIndicesLookupEagerly) { + indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup(dataStreamMetadata, indices)); + } else { + indicesLookup = null; + } - 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. @@ -1530,14 +1556,14 @@ public Metadata build() { String[] visibleClosedIndicesArray = visibleClosedIndices.toArray(Strings.EMPTY_ARRAY); return new Metadata(clusterUUID, clusterUUIDCommitted, version, coordinationMetadata, transientSettings, persistentSettings, - hashesOfConsistentSettings, indices.build(), templates.build(), customs.build(), allIndicesArray, visibleIndicesArray, + hashesOfConsistentSettings, indices, templates.build(), customs.build(), allIndicesArray, visibleIndicesArray, allOpenIndicesArray, visibleOpenIndicesArray, allClosedIndicesArray, visibleClosedIndicesArray, indicesLookup); } - private SortedMap buildIndicesLookup() { + static SortedMap buildIndicesLookup(DataStreamMetadata dataStreamMetadata, + ImmutableOpenMap indices) { SortedMap indicesLookup = new TreeMap<>(); Map indexToDataStreamLookup = new HashMap<>(); - DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE); // If there are no indices, then skip data streams. This happens only when metadata is read from disk if (dataStreamMetadata != null && indices.size() > 0) { Map> dataStreamToAliasLookup = new HashMap<>(); @@ -1579,9 +1605,7 @@ private SortedMap buildIndicesLookup() { } Map> aliasToIndices = new HashMap<>(); - for (var cursor : indices.values()) { - IndexMetadata indexMetadata = cursor.value; - + for (var indexMetadata : indices.values()) { IndexAbstraction.Index index; DataStream parent = indexToDataStreamLookup.get(indexMetadata.getIndex().getName()); if (parent != null) { @@ -1600,13 +1624,8 @@ private SortedMap buildIndicesLookup() { for (var aliasCursor : indexMetadata.getAliases()) { AliasMetadata aliasMetadata = aliasCursor.value; - aliasToIndices.compute(aliasMetadata.getAlias(), (aliasName, indices) -> { - if (indices == null) { - indices = new ArrayList<>(); - } - indices.add(indexMetadata); - return indices; - }); + List aliasIndices = aliasToIndices.computeIfAbsent(aliasMetadata.getAlias(), k -> new ArrayList<>()); + aliasIndices.add(indexMetadata); } } @@ -1616,6 +1635,7 @@ private SortedMap buildIndicesLookup() { assert existing == null : "duplicate for " + entry.getKey(); } + validateDataStreams(indicesLookup, dataStreamMetadata); return indicesLookup; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java index 74df72ebfe6a8..25a9290a4d469 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java @@ -89,7 +89,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) { .put(IndexMetadata.builder(targetIndexMetadata) .putCustom(ILM_CUSTOM_METADATA_KEY, relevantTargetCustomData.build().asMap())); - return ClusterState.builder(clusterState).metadata(newMetadata).build(); + return ClusterState.builder(clusterState).metadata(newMetadata.build(false)).build(); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopySettingsStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopySettingsStep.java index 6f51ee2dd6097..c4bedc0eb7230 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopySettingsStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopySettingsStep.java @@ -87,7 +87,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) { .put(IndexMetadata.builder(targetIndexMetadata) .settingsVersion(targetIndexMetadata.getSettingsVersion() + 1) .settings(settings)); - return ClusterState.builder(clusterState).metadata(newMetaData).build(); + return ClusterState.builder(clusterState).metadata(newMetaData.build(false)).build(); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java index 249b56c9a2b93..47492df01b123 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java @@ -93,7 +93,8 @@ public ClusterState performAction(Index index, ClusterState clusterState) { return ClusterState.builder(clusterState) .metadata(Metadata.builder(clusterState.getMetadata()) .put(IndexMetadata.builder(indexMetaData) - .putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap()))) + .putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap())) + .build(false)) .build(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateUniqueIndexNameStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateUniqueIndexNameStep.java index af1e26767b72f..216cdee02efb6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateUniqueIndexNameStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateUniqueIndexNameStep.java @@ -90,7 +90,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) { IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexMetadata); indexMetadataBuilder.putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap()); - newClusterStateBuilder.metadata(Metadata.builder(clusterState.getMetadata()).put(indexMetadataBuilder)); + newClusterStateBuilder.metadata(Metadata.builder(clusterState.getMetadata()).put(indexMetadataBuilder).build(false)); return newClusterStateBuilder.build(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java index 7830b4449d556..d541106dae5d0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java @@ -68,7 +68,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) { indexMetadataBuilder.putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap()); newClusterStateBuilder.metadata( - Metadata.builder(clusterState.getMetadata()).put(indexMetadataBuilder) + Metadata.builder(clusterState.getMetadata()).put(indexMetadataBuilder).build(false) ); return newClusterStateBuilder.build(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/PhaseCacheManagement.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/PhaseCacheManagement.java index 601aab492d5b2..cd085e736f7b6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/PhaseCacheManagement.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/PhaseCacheManagement.java @@ -51,7 +51,7 @@ public static ClusterState refreshPhaseDefinition(final ClusterState state, fina final IndexMetadata idxMeta = state.metadata().index(index); Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()); refreshPhaseDefinition(metadataBuilder, idxMeta, updatedPolicy); - return ClusterState.builder(state).metadata(metadataBuilder).build(); + return ClusterState.builder(state).metadata(metadataBuilder.build(false)).build(); } /** @@ -109,7 +109,7 @@ public static ClusterState updateIndicesForPolicy(final ClusterState state, fina final LifecyclePolicyMetadata newPolicy, XPackLicenseState licenseState) { Metadata.Builder mb = Metadata.builder(state.metadata()); if (updateIndicesForPolicy(mb, state, xContentRegistry, client, oldPolicy, newPolicy, licenseState)) { - return ClusterState.builder(state).metadata(mb).build(); + return ClusterState.builder(state).metadata(mb.build(false)).build(); } return state; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ReplaceDataStreamBackingIndexStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ReplaceDataStreamBackingIndexStep.java index c1fc7f85ba65b..89e49038b1dd7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ReplaceDataStreamBackingIndexStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ReplaceDataStreamBackingIndexStep.java @@ -97,7 +97,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) { Metadata.Builder newMetaData = Metadata.builder(clusterState.getMetadata()) .put(dataStream.getDataStream().replaceBackingIndex(index, targetIndexMetadata.getIndex())); - return ClusterState.builder(clusterState).metadata(newMetaData).build(); + return ClusterState.builder(clusterState).metadata(newMetaData.build(false)).build(); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/UpdateRolloverLifecycleDateStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/UpdateRolloverLifecycleDateStep.java index 1aecedfdc5ae9..cc942fec2a453 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/UpdateRolloverLifecycleDateStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/UpdateRolloverLifecycleDateStep.java @@ -70,7 +70,7 @@ public ClusterState performAction(Index index, ClusterState currentState) { IndexMetadata.Builder newIndexMetadata = IndexMetadata.builder(indexMetadata); newIndexMetadata.putCustom(ILM_CUSTOM_METADATA_KEY, newLifecycleState.build().asMap()); return ClusterState.builder(currentState).metadata(Metadata.builder(currentState.metadata()) - .put(newIndexMetadata)).build(); + .put(newIndexMetadata).build(false)).build(); } private static String getRolloverTarget(Index index, ClusterState currentState) { diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java index cb6f4a45faef3..4e8a137cce29c 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleRunner.java @@ -67,6 +67,8 @@ class IndexLifecycleRunner { builder.failure(task, e); } } + // Trigger indices lookup creation and related validation + state.metadata().getIndicesLookup(); return builder.build(state); }; diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java index b738a5415201d..b0eae7d490286 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java @@ -344,7 +344,8 @@ public static ClusterState.Builder newClusterStateWithLifecycleState(Index index ClusterState.Builder newClusterStateBuilder = ClusterState.builder(clusterState); newClusterStateBuilder.metadata(Metadata.builder(clusterState.getMetadata()) .put(IndexMetadata.builder(clusterState.getMetadata().index(index)) - .putCustom(ILM_CUSTOM_METADATA_KEY, lifecycleState.asMap()))); + .putCustom(ILM_CUSTOM_METADATA_KEY, lifecycleState.asMap())) + .build(false)); return newClusterStateBuilder; }