Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Optimization] Cluster State Update Optimization #7853

Merged
merged 24 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7409e4d
Draft Cluster State Update Optimization
sandeshkr419 May 31, 2023
22e435f
Removing explicit skip computation variable for computing all indices…
sandeshkr419 Jun 2, 2023
bae5446
Minor formatting
sandeshkr419 Jun 2, 2023
37ca81d
Add Chengelog
sandeshkr419 Jun 2, 2023
fa1374a
Merge branch 'main' into indicesLookup
sandeshkr419 Jun 2, 2023
62a3f69
Refactor
sandeshkr419 Jun 2, 2023
198a11b
Add Chengelog
sandeshkr419 Jun 2, 2023
ecec7c8
Change customs comparison to data stream comparison only
sandeshkr419 Jun 8, 2023
cc41985
Merge branch 'main' into indicesLookup
sandeshkr419 Jun 8, 2023
35b1c8a
Fix dataStreamPreviousMetadata NPE
sandeshkr419 Jun 8, 2023
01fcd3a
Remove dead code
sandeshkr419 Jun 8, 2023
59af76e
Spotless fix
sandeshkr419 Jun 8, 2023
e347287
Tests fix for NPE
sandeshkr419 Jun 8, 2023
264a435
Utilize previousMetadata to insted of multiple variable copies
sandeshkr419 Jun 19, 2023
7b81fb2
Merge branch 'main' into indicesLookup
sandeshkr419 Jun 19, 2023
b03507f
Merge branch 'main' into indicesLookup
sandeshkr419 Jun 29, 2023
82c78eb
Merge branch 'main' into indicesLookup
sandeshkr419 Jun 29, 2023
00199f9
Remove extra log lines and add test cases
sandeshkr419 Jun 30, 2023
e3cca6f
Refactoring and spotless check
sandeshkr419 Jun 30, 2023
a91edcb
Inclusing tests for metadata comparison in existing test
sandeshkr419 Jun 30, 2023
f16d1ac
Fix test
sandeshkr419 Jun 30, 2023
0e6bf89
Merge branch 'main' into indicesLookup
sandeshkr419 Jul 6, 2023
a9b81f4
Merge branch 'main' into indicesLookup
sandeshkr419 Jul 7, 2023
d0f0135
Merge branch 'main' into indicesLookup
sandeshkr419 Jul 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,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
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:
sandeshkr419 marked this conversation as resolved.
Show resolved Hide resolved
// 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
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() {
sandeshkr419 marked this conversation as resolved.
Show resolved Hide resolved
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());
}
}
}