Skip to content

Improving performance of get data streams API by avoiding getting effective mappings #138948

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.

Sign up for GitHub

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

Merged
masseyke merged 8 commits into from
Dec 3, 2025
+36 −6
Merged

Improving performance of get data streams API by avoiding getting effective mappings #138948

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4607a68
Improving performance of get data streams API by avoiding getting eff…
masseyke Dec 2, 2025
731bc34
Update docs/changelog/138948.yaml
masseyke Dec 2, 2025
8e017eb
[CI] Auto commit changes from spotless
Dec 2, 2025
8bbd8f6
Adding a test that shows that the additional settings providers still…
masseyke Dec 3, 2025
ea33136
updating a comment
masseyke Dec 3, 2025
376211e
fixing test name
masseyke Dec 3, 2025
33c321e
removing flakey test
masseyke Dec 3, 2025
27ecc4a
Merge branch 'main' into get-data-streams-performance
masseyke Dec 3, 2025
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
View file
Open in desktop
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: []
23 changes: 17 additions & 6 deletions ...ams/src/main/java/org/elasticsearch/datastreams/action/TransportGetDataStreamsAction.java
View file
Open in desktop
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,22 @@ 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 only need the
* settings so that we can get some ilm information and the index mode, neither of which come from additional settings
* providers.
Copy link
Contributor

@nielsbauman nielsbauman Dec 3, 2025

Choose a reason for hiding this comment

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

the index mode, neither of which come from additional settings
Comment providers.

I don't think that is true; LogsdbIndexModeSettingsProvider seems to supply an index mode:

elasticsearch/x-pack/plugin/logsdb/src/main/java/org/elasticsearch/xpack/logsdb/LogsdbIndexModeSettingsProvider.java

Line 133 in 4c33d20

additionalSettings.put(IndexSettings.MODE.getKey(), IndexMode.LOGSDB.getName());

Or am I misreading something?

Copy link
Member Author

@masseyke masseyke Dec 3, 2025

Choose a reason for hiding this comment

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

Oh good catch. That complicates things.

Copy link
Member Author

@masseyke masseyke Dec 3, 2025

Choose a reason for hiding this comment

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

I'm thinking then that I'll still call getEffectiveSettings, but a new variant that lets you skip merging all the mappings.

Copy link
Member Author

@masseyke masseyke Dec 3, 2025

Choose a reason for hiding this comment

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

Oh, actually that next line that calls resolveMode actually applies all of the additional settings providers and we get the correct mode anyway. I'll add a test that shows that.

*/
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

@dakrone dakrone Dec 2, 2025

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

@masseyke masseyke Dec 3, 2025

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
14 changes: 14 additions & 0 deletions ...RestTest/resources/rest-api-spec/test/data_stream/280_data_stream_mappings-timeseries.yml
View file
Open in desktop
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 }