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

Track and use dynamic dimensions for routing #102977

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

kkrik-es
Copy link
Contributor

@kkrik-es kkrik-es commented Dec 5, 2023

Dynamic templates allow defining fields with time_series_dimension annotations. When these fields are referenced in dynamic template specs at indexing time, they can be used for routing instead of the routing path; the latter can be empty in case all dimensions are dynamically specified.

Related to #98384

Dynamic templates allow defining fields with `time_series_dimension`
annotations. When these fields are referenced in dynamic template specs
at indexing time, they can be used for routing instead of the routing
path; the latter can be empty in case all dimensions are dynamically
specified.

Related to elastic#98384
@kkrik-es kkrik-es added >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :StorageEngine/TSDB You know, for Metrics labels Dec 5, 2023
@kkrik-es kkrik-es self-assigned this Dec 5, 2023
@elasticsearchmachine
Copy link
Collaborator

Hi @kkrik-es, I've created a changelog YAML for you.

kkrik-es and others added 5 commits December 5, 2023 14:43
@@ -113,11 +113,23 @@ public Settings getAdditionalIndexSettings(
builder.put(IndexSettings.TIME_SERIES_START_TIME.getKey(), FORMATTER.format(start));
builder.put(IndexSettings.TIME_SERIES_END_TIME.getKey(), FORMATTER.format(end));

if (allSettings.hasValue(IndexMetadata.INDEX_ROUTING_PATH.getKey()) == false
boolean useDynamicDimensions = allSettings.getAsBoolean(IndexMetadata.TIME_SERIES_DYNAMIC_TEMPLATES.getKey(), false);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a else if() just for dynamic dimensions?

Copy link
Contributor Author

@kkrik-es kkrik-es Dec 8, 2023

Choose a reason for hiding this comment

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

I think we want to retrieve dynamic dimensions even when there's a routing path, to support migrating existing indexes to the new functionality.

*/
private List<String> findRoutingPaths(String indexName, Settings allSettings, List<CompressedXContent> combinedTemplateMappings) {
private void findRoutingPathsAndDimensions(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe keep the existing findRoutingPaths method unchanged and create a findDimensions(...) method?
This keeps the two modes (index routing path & dynamic dimensions) separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've splitted this in three functions to reuse the MappingService, ptal.

@@ -233,32 +269,57 @@ public void collectSearchShards(String routing, IntConsumer consumer) {
}
}

public static class ExtractFromSource extends IndexRouting {
public static class RoutingPathMatching extends IndexRouting {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe keep the name the same. In both ways we keep extracting routing information from source and this keeps the diff of this change smaller.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, done.

@kkrik-es
Copy link
Contributor Author

kkrik-es commented Dec 7, 2023

@elasticsearchmachine run elasticsearch-ci/part-1

@kkrik-es
Copy link
Contributor Author

kkrik-es commented Dec 8, 2023

@elasticsearchmachine run elasticsearch-ci/8.11.2 / bwc-snapshots

…ting' into prototype/dynamic_dimensions_setting
@@ -8,4 +8,4 @@
<option name="AUTO_RESTART" value="true" />
<method v="2" />
</configuration>
</component>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unintended, can't revert :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants