Skip to content

Commit

Permalink
Allow indices lookup to be built lazily (#78745)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
martijnvg authored Oct 7, 2021
1 parent 9fb6888 commit 877e722
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public interface NonRestorableCustom extends Custom {
private final String[] allClosedIndices;
private final String[] visibleClosedIndices;

private final SortedMap<String, IndexAbstraction> indicesLookup;
private SortedMap<String, IndexAbstraction> indicesLookup;

Metadata(String clusterUUID, boolean clusterUUIDCommitted, long version, CoordinationMetadata coordinationMetadata,
Settings transientSettings, Settings persistentSettings, DiffableStringMap hashesOfConsistentSettings,
Expand Down Expand Up @@ -304,6 +304,11 @@ public boolean equalsAliases(Metadata other) {
}

public SortedMap<String, IndexAbstraction> getIndicesLookup() {
if (indicesLookup != null) {
return indicesLookup;
}
DataStreamMetadata dataStreamMetadata = custom(DataStreamMetadata.TYPE);
indicesLookup = Collections.unmodifiableSortedMap(Builder.buildIndicesLookup(dataStreamMetadata, indices));
return indicesLookup;
}

Expand Down Expand Up @@ -1430,7 +1435,22 @@ public Builder generateClusterUuidIfNeeded() {
return this;
}

/**
* @return a new <code>Metadata</code> 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 <code>Metdata</code> 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 <code>Metadata</code> 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.
Expand Down Expand Up @@ -1514,9 +1534,15 @@ public Metadata build() {
"were found [" + Strings.collectionToCommaDelimitedString(duplicates) + "]");
}

SortedMap<String, IndexAbstraction> indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup());
ImmutableOpenMap<String, IndexMetadata> indices = this.indices.build();

SortedMap<String, IndexAbstraction> 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.
Expand All @@ -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<String, IndexAbstraction> buildIndicesLookup() {
static SortedMap<String, IndexAbstraction> buildIndicesLookup(DataStreamMetadata dataStreamMetadata,
ImmutableOpenMap<String, IndexMetadata> indices) {
SortedMap<String, IndexAbstraction> indicesLookup = new TreeMap<>();
Map<String, DataStream> 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<String, List<String>> dataStreamToAliasLookup = new HashMap<>();
Expand Down Expand Up @@ -1579,9 +1605,7 @@ private SortedMap<String, IndexAbstraction> buildIndicesLookup() {
}

Map<String, List<IndexMetadata>> 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) {
Expand All @@ -1600,13 +1624,8 @@ private SortedMap<String, IndexAbstraction> 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<IndexMetadata> aliasIndices = aliasToIndices.computeIfAbsent(aliasMetadata.getAlias(), k -> new ArrayList<>());
aliasIndices.add(indexMetadata);
}
}

Expand All @@ -1616,6 +1635,7 @@ private SortedMap<String, IndexAbstraction> buildIndicesLookup() {
assert existing == null : "duplicate for " + entry.getKey();
}

validateDataStreams(indicesLookup, dataStreamMetadata);
return indicesLookup;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ class IndexLifecycleRunner {
builder.failure(task, e);
}
}
// Trigger indices lookup creation and related validation
state.metadata().getIndicesLookup();
return builder.build(state);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 877e722

Please sign in to comment.