Skip to content

Commit

Permalink
Reuse previous indices lookup when possible (#79004)
Browse files Browse the repository at this point in the history 
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 to #77888
  • Loading branch information
@martijnvg
martijnvg committed Oct 26, 2021
1 parent 5ee9bde commit bada1ba
Show file tree
Hide file tree
Showing 12 changed files with 223 additions and 14 deletions.
49 changes: 49 additions & 0 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstraction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,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 @@ -322,6 +339,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 @@ -383,6 +418,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);
}
}

}
69 changes: 63 additions & 6 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1038,15 +1038,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 @@ -1057,13 +1060,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 @@ -1075,10 +1082,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 @@ -1097,16 +1131,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 @@ -1187,6 +1227,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 @@ -1196,11 +1238,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 @@ -1217,6 +1263,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 @@ -1255,6 +1303,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 @@ -1291,6 +1341,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 @@ -1538,10 +1590,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
4 changes: 3 additions & 1 deletion server/src/main/java/org/elasticsearch/cluster/routing/allocation/IndexMetadataUpdater.java
View file
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
2 changes: 2 additions & 0 deletions server/src/main/java/org/elasticsearch/cluster/service/MasterService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ private ClusterState patchVersions(ClusterState previousClusterState, ClusterTas

if (previousClusterState != newClusterState) {
// only the master controls the version numbers
final var previousIndicesLookup = newClusterState.metadata().getIndicesLookup();
Builder builder = incrementVersion(newClusterState);
if (previousClusterState.routingTable() != newClusterState.routingTable()) {
builder.routingTable(RoutingTable.builder(newClusterState.routingTable())
Expand All @@ -378,6 +379,7 @@ private ClusterState patchVersions(ClusterState previousClusterState, ClusterTas
}

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

return newClusterState;
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/cluster/ClusterChangedEventTests.java
View file
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
99 changes: 99 additions & 0 deletions server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.xcontent.XContentParser;
Expand Down Expand Up @@ -57,8 +58,10 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.hamcrest.Matchers.startsWith;

public class MetadataTests extends ESTestCase {
Expand Down Expand Up @@ -1520,6 +1523,102 @@ public void testDataStreamWriteRemoveDataStream() {
assertThat(metadata.dataStreamAliases().get("logs-postgres").getDataStreams(), containsInAnyOrder("logs-postgres-replicated"));
}

public void testReuseIndicesLookup() {
String indexName = "my-index";
String aliasName = "my-alias";
String dataStreamName = "logs-mysql-prod";
String dataStreamAliasName = "logs-mysql";
Metadata previous = Metadata.builder().build();

// Things that should change indices lookup
{
Metadata.Builder builder = Metadata.builder(previous);
IndexMetadata idx = DataStreamTestHelper.createFirstBackingIndex(dataStreamName).build();
builder.put(idx, true);
DataStream dataStream = new DataStream(dataStreamName, new DataStream.TimestampField("@timestamp"), List.of(idx.getIndex()));
builder.put(dataStream);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), not(sameInstance(metadata.getIndicesLookup())));
previous = metadata;
}
{
Metadata.Builder builder = Metadata.builder(previous);
builder.put(dataStreamAliasName, dataStreamName, false, null);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), not(sameInstance(metadata.getIndicesLookup())));
previous = metadata;
}
{
Metadata.Builder builder = Metadata.builder(previous);
builder.put(dataStreamAliasName, dataStreamName, true, null);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), not(sameInstance(metadata.getIndicesLookup())));
previous = metadata;
}
{
Metadata.Builder builder = Metadata.builder(previous);
builder.put(IndexMetadata.builder(indexName)
.settings(settings(Version.CURRENT)).creationDate(randomNonNegativeLong())
.numberOfShards(1).numberOfReplicas(0));
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), not(sameInstance(metadata.getIndicesLookup())));
previous = metadata;
}
{
Metadata.Builder builder = Metadata.builder(previous);
IndexMetadata.Builder imBuilder = IndexMetadata.builder(builder.get(indexName));
imBuilder.putAlias(AliasMetadata.builder(aliasName).build());
builder.put(imBuilder);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), not(sameInstance(metadata.getIndicesLookup())));
previous = metadata;
}
{
Metadata.Builder builder = Metadata.builder(previous);
IndexMetadata.Builder imBuilder = IndexMetadata.builder(builder.get(indexName));
imBuilder.putAlias(AliasMetadata.builder(aliasName).writeIndex(true).build());
builder.put(imBuilder);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), not(sameInstance(metadata.getIndicesLookup())));
previous = metadata;
}
{
Metadata.Builder builder = Metadata.builder(previous);
IndexMetadata.Builder imBuilder = IndexMetadata.builder(builder.get(indexName));
Settings.Builder sBuilder = Settings.builder()
.put(builder.get(indexName).getSettings())
.put(IndexMetadata.INDEX_HIDDEN_SETTING.getKey(), true);
imBuilder.settings(sBuilder.build());
builder.put(imBuilder);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), not(sameInstance(metadata.getIndicesLookup())));
previous = metadata;
}

// Things that shouldn't change indices lookup
{
Metadata.Builder builder = Metadata.builder(previous);
IndexMetadata.Builder imBuilder = IndexMetadata.builder(builder.get(indexName));
imBuilder.numberOfReplicas(2);
builder.put(imBuilder);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), sameInstance(metadata.getIndicesLookup()));
previous = metadata;
}
{
Metadata.Builder builder = Metadata.builder(previous);
IndexMetadata.Builder imBuilder = IndexMetadata.builder(builder.get(indexName));
Settings.Builder sBuilder = Settings.builder()
.put(builder.get(indexName).getSettings())
.put(IndexSettings.DEFAULT_FIELD_SETTING.getKey(), "val");
imBuilder.settings(sBuilder.build());
builder.put(imBuilder);
Metadata metadata = builder.build();
assertThat(previous.getIndicesLookup(), sameInstance(metadata.getIndicesLookup()));
previous = metadata;
}
}

public static Metadata randomMetadata() {
return randomMetadata(1);
}
Expand Down

0 comments on commit bada1ba

Please sign in to comment.