Skip to content

Optimize loading mappings when determining synthetic source usage and whether host.name can be sorted on #120055

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

Merged

Conversation

martijnvg
Copy link
Member

@martijnvg martijnvg commented Jan 13, 2025

Optimize mapping service usages in when determining synthetic source usage and assessing whether host.name field can be used for index sorting. This is done by excluding the part of the mapping we're not interested in.

This replaces #119935 and addresses the problem differently, by just loading the parts of the mapping LogsdbIndexModeSettingsProvider is interested in and by this lowering the overhead of parsing/merging mappings that has been reported via #119552

…xModeSettingsProvider

Optimize mapping service usages in when determining synthetic source usage and assessing whether host.name field can be used for index sorting. This is done by excluding the part of the mapping we're not interested in.
@martijnvg martijnvg added >enhancement :StorageEngine/Logs You know, for Logs :StorageEngine/Mapping The storage related side of mappings v8.18.0 labels Jan 13, 2025
@martijnvg martijnvg marked this pull request as ready for review January 13, 2025 15:59
@elasticsearchmachine
Copy link
Collaborator

Hi @martijnvg, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@martijnvg martijnvg changed the title Skip large parts of mappings when loading MapperService in LogsdbInde xModeSettingsProvider Optimize loading mappings when determining synthetic source usage and whether host.name can be sorted on Jan 13, 2025
@martijnvg martijnvg removed the :StorageEngine/Mapping The storage related side of mappings label Jan 13, 2025
for (CompressedXContent mappingSource : combinedTemplateMappings) {
var ref = mappingSource.compressedReference();
var map = XContentHelper.convertToMap(ref, true, XContentType.JSON, MAPPING_INCLUDES, Set.of()).v2();
processedTemplateMappings.add(new CompressedXContent(map));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fairly cryptic. Can you add a comment above explaining the difference between processedTemplateMappings and combinedTemplateMappings?

@kkrik-es
Copy link
Contributor

Do we have enough testing coverage in the provider to capture all the interesting use cases where templates contain the interesting settings and fields?

@martijnvg martijnvg requested a review from a team as a code owner January 14, 2025 14:56
@martijnvg martijnvg force-pushed the optimize_LogsdbIndexModeSettingsProvider_2 branch from 97e4775 to ace4d23 Compare January 14, 2025 14:59
@martijnvg
Copy link
Member Author

Do we have enough testing coverage in the provider to capture all the interesting use cases where templates contain the interesting settings and fields?

I think the unit test coverage in LogsdbIndexModeSettingsProviderTests is sufficient?

@martijnvg martijnvg removed the request for review from a team January 14, 2025 15:00
@martijnvg martijnvg requested a review from kkrik-es January 15, 2025 10:40
Copy link
Contributor

@kkrik-es kkrik-es left a comment

Choose a reason for hiding this comment

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

Please double check testing coverage.

@martijnvg martijnvg added the auto-backport Automatically create backport pull requests when merged label Jan 15, 2025
@martijnvg martijnvg enabled auto-merge (squash) January 15, 2025 16:23
@martijnvg martijnvg merged commit 430c9fa into elastic:main Jan 15, 2025
16 checks passed
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >enhancement :StorageEngine/Logs You know, for Logs Team:StorageEngine v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants