Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions docs/changelog/138948.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 138948
summary: Improving performance of get data streams API by avoiding getting effective
mappings
area: Data streams
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.MetadataCreateDataStreamService;
import org.elasticsearch.cluster.metadata.MetadataDataStreamsService;
import org.elasticsearch.cluster.metadata.MetadataIndexTemplateService;
import org.elasticsearch.cluster.metadata.ProjectMetadata;
Expand Down Expand Up @@ -278,12 +279,21 @@ static GetDataStreamAction.Response innerOperation(
} else {
indexTemplate = MetadataIndexTemplateService.findV2Template(state.metadata(), dataStream.getName(), false);
if (indexTemplate != null) {
final Settings settings;
try {
settings = metadataDataStreamsService.getEffectiveSettings(state.metadata(), dataStream);
} catch (IOException e) {
throw new RuntimeException("Failed to get effective settings for data stream: " + dataStream.getName(), e);
}
/*
* Here we intentionally avoid the full MetadataDataStreamService::getEffectiveSettings and instead do a shortcut that
* does not merge all mappings together in order to fetch the settings from additional settings providers. The reason
* is that this code can be called fairly frequently, and we do not need that information here -- we get settings from
* additional settings providers below in resolveMode, and those settings do not require any information from mappings.
*/
ComposableIndexTemplate template = MetadataCreateDataStreamService.lookupTemplateForDataStream(
dataStream.getName(),
state.metadata()
);
Settings templateSettings = MetadataIndexTemplateService.resolveSettings(
template,
state.metadata().componentTemplates()
);
final Settings settings = templateSettings.merge(dataStream.getSettings());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense. Can we also add something like:

assert settings.equals(metadataDataStreamsSettingsService.getEffectiveSettings(state.metadata(), dataStream)) : "these should always match etc etc";

after this line, so that if we ever encountered a situation where a provider changed the settings, then we would fail tests? That way we only pay the cost when asserts are enabled.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that that assertion would work, b/c the providers do change some of the settings.

ilmPolicyName = settings.get(IndexMetadata.LIFECYCLE_NAME);
if (indexMode == null && state.metadata().templatesV2().get(indexTemplate) != null) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.put_data_stream_mappings:
Expand Down Expand Up @@ -78,6 +79,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings.properties.name.type: "keyword" }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }
- set: { data_streams.0.indices.0.index_name: oldIndexName }
- set: { data_streams.0.indices.1.index_name: newIndexName }

Expand Down Expand Up @@ -141,6 +143,7 @@
- match: { data_streams.0.name: my-component-only-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.put_data_stream_mappings:
Expand Down Expand Up @@ -202,6 +205,7 @@
- match: { data_streams.0.effective_mappings: null }
- set: { data_streams.0.indices.0.index_name: oldIndexName }
- set: { data_streams.0.indices.1.index_name: newIndexName }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.get_mapping:
Expand Down Expand Up @@ -269,6 +273,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.put_index_template:
Expand Down Expand Up @@ -297,6 +302,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.rollover:
Expand All @@ -320,6 +326,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings: { } }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: standard }
- set: { data_streams.0.indices.0.index_name: oldIndexName }
- set: { data_streams.0.indices.1.index_name: newIndexName }

Expand Down Expand Up @@ -380,6 +387,7 @@
- match: { data_streams.0.name: test-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.rollover:
Expand All @@ -403,6 +411,7 @@
- match: { data_streams.0.name: test-data-stream-1 }
- match: { data_streams.0.mappings: { } }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }
- set: { data_streams.0.indices.0.index_name: oldIndexName }
- set: { data_streams.0.indices.1.index_name: newIndexName }

Expand Down Expand Up @@ -449,6 +458,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.put_index_template:
Expand Down Expand Up @@ -477,6 +487,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.rollover:
Expand All @@ -500,6 +511,7 @@
- match: { data_streams.0.name: my-data-stream-1 }
- match: { data_streams.0.mappings: { } }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: standard }
- set: { data_streams.0.indices.0.index_name: oldIndexName }
- set: { data_streams.0.indices.1.index_name: newIndexName }

Expand Down Expand Up @@ -561,6 +573,7 @@
- match: { data_streams.0.name: test-data-stream-1 }
- match: { data_streams.0.mappings: {} }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }

- do:
indices.rollover:
Expand All @@ -584,5 +597,6 @@
- match: { data_streams.0.name: test-data-stream-1 }
- match: { data_streams.0.mappings: { } }
- match: { data_streams.0.effective_mappings: null }
- match: { data_streams.0.index_mode: time_series }
- set: { data_streams.0.indices.0.index_name: oldIndexName }
- set: { data_streams.0.indices.1.index_name: newIndexName }