Skip to content

Commit

Permalink
[Optimization] Cluster State Update Optimization (opensearch-project#…
Browse files Browse the repository at this point in the history 
…7853)

* Cluster State Update Optimization - Optimize Metadata build() to skip redundant computations of indicesLookup as part of ClusterState build

Signed-off-by: Sandesh Kumar <sandeshkr419@gmail.com>

---------

Signed-off-by: Sandesh Kumar <sandeshkr419@gmail.com>
  • Loading branch information
@sandeshkr419 @baba-devv
sandeshkr419 authored and baba-devv committed Jul 29, 2023
1 parent b4d3542 commit c1f7349
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Update ZSTD default compression level ([#8471](https://github.com/opensearch-project/OpenSearch/pull/8471))
- [Search Pipelines] Pass pipeline creation context to processor factories ([#8164](https://github.com/opensearch-project/OpenSearch/pull/8164))
- Enabling compression levels for zstd and zstd_no_dict ([#8312](https://github.com/opensearch-project/OpenSearch/pull/8312))

- Optimize Metadata build() to skip redundant computations as part of ClusterState build ([#7853](https://github.com/opensearch-project/OpenSearch/pull/7853))

### Deprecated

Expand Down
44 changes: 42 additions & 2 deletions server/src/main/java/org/opensearch/cluster/metadata/Metadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1121,12 +1121,14 @@ public static class Builder {
private final Map<String, IndexMetadata> indices;
private final Map<String, IndexTemplateMetadata> templates;
private final Map<String, Custom> customs;
private final Metadata previousMetadata;

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

Expand All @@ -1141,6 +1143,7 @@ public Builder(Metadata metadata) {
this.indices = new HashMap<>(metadata.indices);
this.templates = new HashMap<>(metadata.templates);
this.customs = new HashMap<>(metadata.customs);
this.previousMetadata = metadata;
}

public Builder put(IndexMetadata.Builder indexMetadataBuilder) {
Expand Down Expand Up @@ -1425,6 +1428,44 @@ public Builder generateClusterUuidIfNeeded() {
}

public Metadata build() {
DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE);
DataStreamMetadata previousDataStreamMetadata = (previousMetadata != null)
? (DataStreamMetadata) this.previousMetadata.customs.get(DataStreamMetadata.TYPE)
: null;

boolean recomputeRequiredforIndicesLookups = (previousMetadata == null)
|| (indices.equals(previousMetadata.indices) == false)
|| (previousDataStreamMetadata != null && previousDataStreamMetadata.equals(dataStreamMetadata) == false)
|| (dataStreamMetadata != null && dataStreamMetadata.equals(previousDataStreamMetadata) == false);

return (recomputeRequiredforIndicesLookups == false)
? buildMetadataWithPreviousIndicesLookups()
: buildMetadataWithRecomputedIndicesLookups();
}

protected Metadata buildMetadataWithPreviousIndicesLookups() {
return new Metadata(
clusterUUID,
clusterUUIDCommitted,
version,
coordinationMetadata,
transientSettings,
persistentSettings,
hashesOfConsistentSettings,
indices,
templates,
customs,
Arrays.copyOf(previousMetadata.allIndices, previousMetadata.allIndices.length),
Arrays.copyOf(previousMetadata.visibleIndices, previousMetadata.visibleIndices.length),
Arrays.copyOf(previousMetadata.allOpenIndices, previousMetadata.allOpenIndices.length),
Arrays.copyOf(previousMetadata.visibleOpenIndices, previousMetadata.visibleOpenIndices.length),
Arrays.copyOf(previousMetadata.allClosedIndices, previousMetadata.allClosedIndices.length),
Arrays.copyOf(previousMetadata.visibleClosedIndices, previousMetadata.visibleClosedIndices.length),
Collections.unmodifiableSortedMap(previousMetadata.indicesLookup)
);
}

protected Metadata buildMetadataWithRecomputedIndicesLookups() {
// 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 @@ -1586,8 +1627,7 @@ private SortedMap<String, IndexAbstraction> buildIndicesLookup() {
IndexAbstraction existing = indicesLookup.put(indexMetadata.getIndex().getName(), index);
assert existing == null : "duplicate for " + indexMetadata.getIndex();

for (final AliasMetadata aliasCursor : indexMetadata.getAliases().values()) {
AliasMetadata aliasMetadata = aliasCursor;
for (final AliasMetadata aliasMetadata : indexMetadata.getAliases().values()) {
indicesLookup.compute(aliasMetadata.getAlias(), (aliasName, alias) -> {
if (alias == null) {
return new IndexAbstraction.Alias(aliasMetadata, indexMetadata);
Expand Down
103 changes: 103 additions & 0 deletions server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -76,6 +77,10 @@
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

public class MetadataTests extends OpenSearchTestCase {

Expand Down Expand Up @@ -1364,6 +1369,62 @@ public void testValidateDataStreamsForNullDataStreamMetadata() {
}
}

public void testMetadataBuildInvocations() {
final Metadata previousMetadata = randomMetadata();
Metadata builtMetadata;
Metadata.Builder spyBuilder;

// previous Metadata state was not provided to Builder during assignment - indices lookups should get re-computed
spyBuilder = spy(Metadata.builder());
builtMetadata = spyBuilder.build();
verify(spyBuilder, times(1)).buildMetadataWithRecomputedIndicesLookups();
verify(spyBuilder, times(0)).buildMetadataWithPreviousIndicesLookups();
compareMetadata(Metadata.EMPTY_METADATA, builtMetadata, true, true, false);

// no changes in builder method after initialization from previous Metadata - indices lookups should not be re-computed
spyBuilder = spy(Metadata.builder(previousMetadata));
builtMetadata = spyBuilder.version(previousMetadata.version() + 1).build();
verify(spyBuilder, times(0)).buildMetadataWithRecomputedIndicesLookups();
verify(spyBuilder, times(1)).buildMetadataWithPreviousIndicesLookups();
compareMetadata(previousMetadata, builtMetadata, true, true, true);
reset(spyBuilder);

// adding new index - all indices lookups should get re-computed
spyBuilder = spy(Metadata.builder(previousMetadata));
String index = "new_index_" + randomAlphaOfLength(3);
builtMetadata = spyBuilder.indices(
Collections.singletonMap(
index,
IndexMetadata.builder(index).settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(1).build()
)
).build();
verify(spyBuilder, times(1)).buildMetadataWithRecomputedIndicesLookups();
verify(spyBuilder, times(0)).buildMetadataWithPreviousIndicesLookups();
compareMetadata(previousMetadata, builtMetadata, false, true, false);
reset(spyBuilder);

// adding new templates - indices lookups should not get recomputed
spyBuilder = spy(Metadata.builder(previousMetadata));
builtMetadata = spyBuilder.put("component_template_new_" + randomAlphaOfLength(3), ComponentTemplateTests.randomInstance())
.put("index_template_v2_new_" + randomAlphaOfLength(3), ComposableIndexTemplateTests.randomInstance())
.build();
verify(spyBuilder, times(0)).buildMetadataWithRecomputedIndicesLookups();
verify(spyBuilder, times(1)).buildMetadataWithPreviousIndicesLookups();
compareMetadata(previousMetadata, builtMetadata, true, false, false);
reset(spyBuilder);

// adding new data stream - indices lookups should get re-computed
spyBuilder = spy(Metadata.builder(previousMetadata));
DataStream dataStream = DataStreamTests.randomInstance();
for (Index backingIndex : dataStream.getIndices()) {
spyBuilder.put(DataStreamTestHelper.getIndexMetadataBuilderForIndex(backingIndex));
}
builtMetadata = spyBuilder.put(dataStream).version(previousMetadata.version() + 1).build();
verify(spyBuilder, times(1)).buildMetadataWithRecomputedIndicesLookups();
verify(spyBuilder, times(0)).buildMetadataWithPreviousIndicesLookups();
compareMetadata(previousMetadata, builtMetadata, false, true, true);
}

public static Metadata randomMetadata() {
Metadata.Builder md = Metadata.builder()
.put(buildIndexMetadata("index", "alias", randomBoolean() ? null : randomBoolean()).build(), randomBoolean())
Expand Down Expand Up @@ -1435,4 +1496,46 @@ private static class CreateIndexResult {
this.metadata = metadata;
}
}

private static void compareMetadata(
final Metadata previousMetadata,
final Metadata newMetadata,
final boolean compareIndicesLookups,
final boolean compareTemplates,
final boolean checkVersionIncrement
) {
assertEquals(previousMetadata.clusterUUID(), newMetadata.clusterUUID());
assertEquals(previousMetadata.clusterUUIDCommitted(), newMetadata.clusterUUIDCommitted());
assertEquals(previousMetadata.coordinationMetadata(), newMetadata.coordinationMetadata());
assertEquals(previousMetadata.settings(), newMetadata.settings());
assertEquals(previousMetadata.transientSettings(), newMetadata.transientSettings());
assertEquals(previousMetadata.persistentSettings(), newMetadata.persistentSettings());
assertEquals(previousMetadata.hashesOfConsistentSettings(), newMetadata.hashesOfConsistentSettings());

if (compareIndicesLookups == true) {
assertEquals(previousMetadata.indices(), newMetadata.indices());
assertEquals(previousMetadata.getConcreteAllIndices(), newMetadata.getConcreteAllIndices());
assertEquals(previousMetadata.getConcreteAllClosedIndices(), newMetadata.getConcreteAllClosedIndices());
assertEquals(previousMetadata.getConcreteAllOpenIndices(), newMetadata.getConcreteAllOpenIndices());
assertEquals(previousMetadata.getConcreteVisibleIndices(), newMetadata.getConcreteVisibleIndices());
assertEquals(previousMetadata.getConcreteVisibleClosedIndices(), newMetadata.getConcreteVisibleClosedIndices());
assertEquals(previousMetadata.getConcreteVisibleOpenIndices(), newMetadata.getConcreteVisibleOpenIndices());
assertEquals(previousMetadata.getIndicesLookup(), newMetadata.getIndicesLookup());
assertEquals(previousMetadata.getCustoms().get(DataStreamMetadata.TYPE), newMetadata.getCustoms().get(DataStreamMetadata.TYPE));
}

if (compareTemplates == true) {
assertEquals(previousMetadata.templates(), newMetadata.templates());
assertEquals(previousMetadata.templatesV2(), newMetadata.templatesV2());
assertEquals(previousMetadata.componentTemplates(), newMetadata.componentTemplates());
}

if (compareIndicesLookups == true && compareTemplates == true) {
assertEquals(previousMetadata.getCustoms(), newMetadata.getCustoms());
}

if (checkVersionIncrement == true) {
assertEquals(previousMetadata.version() + 1, newMetadata.version());
}
}
}

0 comments on commit c1f7349

Please sign in to comment.