Skip to content

Commit

Permalink
Reuse previous indices lookup when possible (#79804)
Browse files Browse the repository at this point in the history
Backporting #79004 to 7.16 branch.

In cases when indices, aliases and data streams aren't modified then
the indices lookup can be reused.

For example in:
* The IndexMetadataUpdater#applyChanges(...) method builds a new metadata
instance, but only primary term or insync allocations may be updated.
No new indices, aliases or data streams are added, so re-building indices
lookup is not necessary.
* MasterService#patchVersions

Additionally the logic that checks when indices lookup can be reused,
this logic also checks the hidden and system flags of indices/datastreams.

In clusters with many indices the cost of building indices lookup is
non-neglectable and should be avoided in this case.

Closes #78980
Partially addresses #77888
  • Loading branch information
martijnvg authored Oct 26, 2021
1 parent 5b7211f commit 7e9b8cd
Show file tree
Hide file tree
Showing 11 changed files with 225 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,23 @@ public boolean isSystem() {
public List<String> getAliases() {
return aliases;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ConcreteIndex that = (ConcreteIndex) o;
return isHidden == that.isHidden &&
isSystem == that.isSystem &&
concreteIndexName.equals(that.concreteIndexName) &&
Objects.equals(aliases, that.aliases) &&
Objects.equals(dataStream, that.dataStream);
}

@Override
public int hashCode() {
return Objects.hash(concreteIndexName, isHidden, isSystem, aliases, dataStream);
}
}

/**
Expand Down Expand Up @@ -302,6 +319,24 @@ private void validateAliasProperties(List<IndexMetadata> referenceIndexMetadatas
private boolean isNonEmpty(List<IndexMetadata> idxMetas) {
return (Objects.isNull(idxMetas) || idxMetas.isEmpty()) == false;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Alias alias = (Alias) o;
return isHidden == alias.isHidden &&
isSystem == alias.isSystem &&
dataStreamAlias == alias.dataStreamAlias &&
aliasName.equals(alias.aliasName) &&
referenceIndexMetadatas.equals(alias.referenceIndexMetadatas) &&
Objects.equals(writeIndex, alias.writeIndex);
}

@Override
public int hashCode() {
return Objects.hash(aliasName, referenceIndexMetadatas, writeIndex, isHidden, isSystem, dataStreamAlias);
}
}

class DataStream implements IndexAbstraction {
Expand Down Expand Up @@ -363,6 +398,20 @@ public List<String> getAliases() {
public org.elasticsearch.cluster.metadata.DataStream getDataStream() {
return dataStream;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
DataStream that = (DataStream) o;
return dataStream.equals(that.dataStream) &&
Objects.equals(referencedByDataStreamAliases, that.referencedByDataStreamAliases);
}

@Override
public int hashCode() {
return Objects.hash(dataStream, referencedByDataStreamAliases);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1160,15 +1160,18 @@ public static class Builder {
private final ImmutableOpenMap.Builder<String, IndexTemplateMetadata> templates;
private final ImmutableOpenMap.Builder<String, Custom> customs;

private SortedMap<String, IndexAbstraction> previousIndicesLookup;

public Builder() {
clusterUUID = UNKNOWN_CLUSTER_UUID;
indices = ImmutableOpenMap.builder();
templates = ImmutableOpenMap.builder();
customs = ImmutableOpenMap.builder();
indexGraveyard(IndexGraveyard.builder().build()); // create new empty index graveyard to initialize
previousIndicesLookup = null;
}

public Builder(Metadata metadata) {
Builder(Metadata metadata) {
this.clusterUUID = metadata.clusterUUID;
this.clusterUUIDCommitted = metadata.clusterUUIDCommitted;
this.coordinationMetadata = metadata.coordinationMetadata;
Expand All @@ -1179,13 +1182,17 @@ public Builder(Metadata metadata) {
this.indices = ImmutableOpenMap.builder(metadata.indices);
this.templates = ImmutableOpenMap.builder(metadata.templates);
this.customs = ImmutableOpenMap.builder(metadata.customs);
previousIndicesLookup = metadata.getIndicesLookup();
}

public Builder put(IndexMetadata.Builder indexMetadataBuilder) {
// we know its a new one, increment the version and store
indexMetadataBuilder.version(indexMetadataBuilder.version() + 1);
IndexMetadata indexMetadata = indexMetadataBuilder.build();
indices.put(indexMetadata.getIndex().getName(), indexMetadata);
IndexMetadata previous = indices.put(indexMetadata.getIndex().getName(), indexMetadata);
if (unsetPreviousIndicesLookup(previous, indexMetadata)) {
previousIndicesLookup = null;
}
return this;
}

Expand All @@ -1197,10 +1204,37 @@ public Builder put(IndexMetadata indexMetadata, boolean incrementVersion) {
if (incrementVersion) {
indexMetadata = IndexMetadata.builder(indexMetadata).version(indexMetadata.getVersion() + 1).build();
}
indices.put(indexMetadata.getIndex().getName(), indexMetadata);
IndexMetadata previous = indices.put(indexMetadata.getIndex().getName(), indexMetadata);
if (unsetPreviousIndicesLookup(previous, indexMetadata)) {
previousIndicesLookup = null;
}
return this;
}

boolean unsetPreviousIndicesLookup(IndexMetadata previous, IndexMetadata current) {
if (previous == null) {
return true;
}

if (previous.getAliases().equals(current.getAliases()) == false) {
return true;
}

if (previous.isHidden() != current.isHidden()) {
return true;
}

if (previous.isSystem() != current.isSystem()) {
return true;
}

if (previous.getState() != current.getState()) {
return true;
}

return false;
}

public IndexMetadata get(String index) {
return indices.get(index);
}
Expand All @@ -1219,16 +1253,22 @@ public IndexMetadata getSafe(Index index) {
}

public Builder remove(String index) {
previousIndicesLookup = null;

indices.remove(index);
return this;
}

public Builder removeAllIndices() {
previousIndicesLookup = null;

indices.clear();
return this;
}

public Builder indices(ImmutableOpenMap<String, IndexMetadata> indices) {
previousIndicesLookup = null;

this.indices.putAll(indices);
return this;
}
Expand Down Expand Up @@ -1309,6 +1349,8 @@ public Builder removeIndexTemplate(String name) {
}

public DataStream dataStream(String dataStreamName) {
previousIndicesLookup = null;

DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) customs.get(DataStreamMetadata.TYPE);
if (dataStreamMetadata != null) {
return dataStreamMetadata.dataStreams().get(dataStreamName);
Expand All @@ -1318,11 +1360,15 @@ public DataStream dataStream(String dataStreamName) {
}

public Builder dataStreams(Map<String, DataStream> dataStreams, Map<String, DataStreamAlias> dataStreamAliases) {
previousIndicesLookup = null;

this.customs.put(DataStreamMetadata.TYPE, new DataStreamMetadata(dataStreams, dataStreamAliases));
return this;
}

public Builder put(DataStream dataStream) {
previousIndicesLookup = null;

Objects.requireNonNull(dataStream, "it is invalid to add a null data stream");
Map<String, DataStream> existingDataStreams =
Optional.ofNullable((DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE))
Expand All @@ -1339,6 +1385,8 @@ public Builder put(DataStream dataStream) {
}

public boolean put(String aliasName, String dataStream, Boolean isWriteDataStream, String filter) {
previousIndicesLookup = null;

Map<String, DataStream> existingDataStream =
Optional.ofNullable((DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE))
.map(dsmd -> new HashMap<>(dsmd.dataStreams()))
Expand Down Expand Up @@ -1377,6 +1425,8 @@ public boolean put(String aliasName, String dataStream, Boolean isWriteDataStrea
}

public Builder removeDataStream(String name) {
previousIndicesLookup = null;

Map<String, DataStream> existingDataStreams =
Optional.ofNullable((DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE))
.map(dsmd -> new HashMap<>(dsmd.dataStreams()))
Expand Down Expand Up @@ -1413,6 +1463,8 @@ public Builder removeDataStream(String name) {
}

public boolean removeDataStreamAlias(String aliasName, String dataStreamName, boolean mustExist) {
previousIndicesLookup = null;

Map<String, DataStreamAlias> dataStreamAliases =
Optional.ofNullable((DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE))
.map(dsmd -> new HashMap<>(dsmd.getDataStreamAliases()))
Expand Down Expand Up @@ -1665,10 +1717,15 @@ public Metadata build(boolean builtIndicesLookupEagerly) {
ImmutableOpenMap<String, IndexMetadata> indices = this.indices.build();

SortedMap<String, IndexAbstraction> indicesLookup;
if (builtIndicesLookupEagerly) {
indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup(dataStreamMetadata, indices));
if (previousIndicesLookup != null) {
assert previousIndicesLookup.equals(buildIndicesLookup(dataStreamMetadata, indices));
indicesLookup = previousIndicesLookup;
} else {
indicesLookup = null;
if (builtIndicesLookupEagerly) {
indicesLookup = Collections.unmodifiableSortedMap(buildIndicesLookup(dataStreamMetadata, indices));
} else {
indicesLookup = null;
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ public Metadata applyChanges(Metadata oldMetadata, RoutingTable newRoutingTable)
}

if (metadataBuilder != null) {
return metadataBuilder.build();
Metadata newMetadata = metadataBuilder.build();
assert oldMetadata.getIndicesLookup() == newMetadata.getIndicesLookup();
return newMetadata;
} else {
return oldMetadata;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.cluster.ClusterStateTaskListener;
import org.elasticsearch.cluster.coordination.ClusterStatePublisher;
import org.elasticsearch.cluster.coordination.FailedToCommitClusterStateException;
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.metadata.ProcessClusterEventTimeoutException;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand Down Expand Up @@ -53,6 +54,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.SortedMap;
import java.util.concurrent.TimeUnit;
import java.util.function.LongSupplier;
import java.util.function.Supplier;
Expand Down Expand Up @@ -374,6 +376,7 @@ private ClusterState patchVersions(ClusterState previousClusterState, ClusterTas

if (previousClusterState != newClusterState) {
// only the master controls the version numbers
final SortedMap<String, IndexAbstraction> previousIndicesLookup = newClusterState.metadata().getIndicesLookup();
Builder builder = incrementVersion(newClusterState);
if (previousClusterState.routingTable() != newClusterState.routingTable()) {
builder.routingTable(RoutingTable.builder(newClusterState.routingTable())
Expand All @@ -384,6 +387,7 @@ private ClusterState patchVersions(ClusterState previousClusterState, ClusterTas
}

newClusterState = builder.build();
assert previousIndicesLookup == newClusterState.metadata().getIndicesLookup();
}

return newClusterState;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ private static ClusterState createNonInitializedState(final int numNodes, final
private static ClusterState nextState(final ClusterState previousState, List<TestCustomMetadata> customMetadataList) {
final ClusterState.Builder builder = ClusterState.builder(previousState);
builder.stateUUID(UUIDs.randomBase64UUID());
Metadata.Builder metadataBuilder = new Metadata.Builder(previousState.metadata());
Metadata.Builder metadataBuilder = Metadata.builder(previousState.metadata());
for (ObjectObjectCursor<String, Metadata.Custom> customMetadata : previousState.metadata().customs()) {
if (customMetadata.value instanceof TestCustomMetadata) {
metadataBuilder.removeCustom(customMetadata.key);
Expand Down
Loading

0 comments on commit 7e9b8cd

Please sign in to comment.