-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Default indexed to false for keyword dimension fields #137029
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| this.dimension = TimeSeriesParams.dimensionParam(m -> toType(m).fieldType().isDimension()).addValidator(v -> { | ||
| if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) { | ||
| if (v && (hasDocValues.getValue() == false)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change will need to be applied to all field mappers that can be dimensions:
BooleanFieldMapper
FlattenedFieldMapper
GeoPointFieldMapper
IpFieldMapper
NumberFieldMapper
ScaledFloatFieldMapper
UnsignedLongFieldMapper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and it will need gating behind a FeatureFlag and IndexVersion too. This is just for experimenting, really.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we at least also add IpFieldMapper? I think that will be important for the experimentation.
|
I ran this against the metricsgen track. The highlights:
Click to expand full rally comparison |
|
I've extracted flame graphs for two queries that have regressed the most: avg_avgot_cpu_load_1m_filtered_by_hosts_5m-main.html avg_avgot_cpu_load_1m_prefix_by_hosts_5m-main.html What both have in common that they're using a two phase iterator in the skippers version. The |
No description provided.