-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Refactoring doc values sparse index enabling for the host.name field
#121751
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
salvatore-campagna
merged 23 commits into
elastic:main
from
salvatore-campagna:feature/hostname-doc-values-sparse-index-setting
Feb 10, 2025
Merged
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
c4ed6c2
feature: enable doc values sparse idnex using an index setting
salvatore-campagna c8e6ce2
fix: refactor check on sorting and remove index param check
salvatore-campagna fee1b5a
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna 7b795ba
fix: relay on default setting value
salvatore-campagna 115e30c
fix: simplify sort check on host.name
salvatore-campagna 598776d
fix: rework logic to conditionally add the new setting
salvatore-campagna a9b76ce
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna efad1cd
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna 85f00a4
fix: make methods static and update javadoc
salvatore-campagna 1317e2d
docs: remove javadocs, method is self-explanatory
salvatore-campagna 0f760e3
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna 0a40c32
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna 2214c97
fix: default true for snapshot builds, false otherwise
salvatore-campagna 7afba8e
fix: gate usage of sparse index with feature flag
salvatore-campagna 531a3e9
fix: udpate test according to default value
salvatore-campagna 066a1d3
test: run both in snapshot and release builds
salvatore-campagna 3015eb2
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna eda9755
not: rename doc values sparse index to doc values skipper
salvatore-campagna 41c6098
fix: delete file
salvatore-campagna 30a0c13
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna 0fc3f4b
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna 9ab1a9a
fix: typo 'Filed' instead of 'Field'
salvatore-campagna 8a405f2
fix: replace assertEquals with if statement
salvatore-campagna File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
357 changes: 186 additions & 171 deletions
357
server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,7 @@ | |
| import static org.apache.lucene.index.IndexWriter.MAX_TERM_LENGTH; | ||
| import static org.elasticsearch.core.Strings.format; | ||
| import static org.elasticsearch.index.IndexSettings.IGNORE_ABOVE_SETTING; | ||
| import static org.elasticsearch.index.IndexSettings.USE_DOC_VALUES_SKIPPER; | ||
|
|
||
| /** | ||
| * A field mapper for keywords. This mapper accepts strings and indexes them as-is. | ||
|
|
@@ -201,6 +202,7 @@ public static final class Builder extends FieldMapper.DimensionBuilder { | |
| private final IndexAnalyzers indexAnalyzers; | ||
| private final ScriptCompiler scriptCompiler; | ||
| private final IndexVersion indexCreatedVersion; | ||
| private final boolean useDocValuesSkipper; | ||
|
|
||
| public Builder(final String name, final MappingParserContext mappingParserContext) { | ||
| this( | ||
|
|
@@ -210,7 +212,8 @@ public Builder(final String name, final MappingParserContext mappingParserContex | |
| IGNORE_ABOVE_SETTING.get(mappingParserContext.getSettings()), | ||
| mappingParserContext.getIndexSettings().getIndexVersionCreated(), | ||
| mappingParserContext.getIndexSettings().getMode(), | ||
| mappingParserContext.getIndexSettings().getIndexSortConfig() | ||
| mappingParserContext.getIndexSettings().getIndexSortConfig(), | ||
| USE_DOC_VALUES_SKIPPER.get(mappingParserContext.getSettings()) | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -221,7 +224,7 @@ public Builder(final String name, final MappingParserContext mappingParserContex | |
| int ignoreAboveDefault, | ||
| IndexVersion indexCreatedVersion | ||
| ) { | ||
| this(name, indexAnalyzers, scriptCompiler, ignoreAboveDefault, indexCreatedVersion, IndexMode.STANDARD, null); | ||
| this(name, indexAnalyzers, scriptCompiler, ignoreAboveDefault, indexCreatedVersion, IndexMode.STANDARD, null, false); | ||
| } | ||
|
|
||
| private Builder( | ||
|
|
@@ -231,7 +234,8 @@ private Builder( | |
| int ignoreAboveDefault, | ||
| IndexVersion indexCreatedVersion, | ||
| IndexMode indexMode, | ||
| IndexSortConfig indexSortConfig | ||
| IndexSortConfig indexSortConfig, | ||
| boolean useDocValuesSkipper | ||
| ) { | ||
| super(name); | ||
| this.indexAnalyzers = indexAnalyzers; | ||
|
|
@@ -268,6 +272,7 @@ private Builder( | |
| }); | ||
| this.indexSortConfig = indexSortConfig; | ||
| this.indexMode = indexMode; | ||
| this.useDocValuesSkipper = useDocValuesSkipper; | ||
| } | ||
|
|
||
| public Builder(String name, IndexVersion indexCreatedVersion) { | ||
|
|
@@ -394,7 +399,13 @@ private KeywordFieldType buildFieldType(MapperBuilderContext context, FieldType | |
|
|
||
| @Override | ||
| public KeywordFieldMapper build(MapperBuilderContext context) { | ||
| FieldType fieldtype = resolveFieldType(indexCreatedVersion, indexSortConfig, indexMode, context.buildFullName(leafName())); | ||
| FieldType fieldtype = resolveFieldType( | ||
| useDocValuesSkipper, | ||
| indexCreatedVersion, | ||
| indexSortConfig, | ||
| indexMode, | ||
| context.buildFullName(leafName()) | ||
| ); | ||
| fieldtype.setOmitNorms(this.hasNorms.getValue() == false); | ||
| fieldtype.setStored(this.stored.getValue()); | ||
| fieldtype.setDocValuesType(this.hasDocValues.getValue() ? DocValuesType.SORTED_SET : DocValuesType.NONE); | ||
|
|
@@ -417,65 +428,40 @@ public KeywordFieldMapper build(MapperBuilderContext context) { | |
| buildFieldType(context, fieldtype), | ||
| builderParams(this, context), | ||
| context.isSourceSynthetic(), | ||
| useDocValuesSkipper, | ||
| this | ||
| ); | ||
| } | ||
|
|
||
| private FieldType resolveFieldType( | ||
| final boolean useDocValuesSkipper, | ||
| final IndexVersion indexCreatedVersion, | ||
| final IndexSortConfig indexSortConfig, | ||
| final IndexMode indexMode, | ||
| final String fullFieldName | ||
| ) { | ||
| if (FieldMapper.DOC_VALUES_SPARSE_INDEX.isEnabled() | ||
| if (useDocValuesSkipper | ||
| && indexCreatedVersion.onOrAfter(IndexVersions.HOSTNAME_DOC_VALUES_SPARSE_INDEX) | ||
| && shouldUseDocValuesSparseIndex(indexSortConfig, indexMode, fullFieldName)) { | ||
| && shouldUseDocValuesSkipper(hasDocValues.getValue(), indexSortConfig, indexMode, fullFieldName)) { | ||
| return new FieldType(Defaults.FIELD_TYPE_WITH_SKIP_DOC_VALUES); | ||
| } | ||
| return new FieldType(Defaults.FIELD_TYPE); | ||
| } | ||
|
|
||
| /** | ||
| * Determines whether to use a sparse index representation for doc values. | ||
| * | ||
| * <p>If the field is explicitly indexed by setting {@code index: true}, we do not use | ||
| * a sparse doc values index but instead rely on the inverted index, as is typically | ||
| * the case for keyword fields.</p> | ||
| * | ||
| * <p>This method checks several conditions to decide if the sparse index format | ||
| * should be applied:</p> | ||
| * | ||
| * <ul> | ||
| * <li>Returns {@code false} immediately if the field is explicitly indexed.</li> | ||
| * <li>Ensures the field is not explicitly configured as indexed (i.e., {@code index} has its default value).</li> | ||
| * <li>Requires doc values to be enabled.</li> | ||
| * <li>Index mode must be {@link IndexMode#LOGSDB}.</li> | ||
| * <li>Field name must be {@code host.name}.</li> | ||
| * <li>The {@code host.name} field must be a primary sort field.</li> | ||
| * </ul> | ||
| * | ||
| * <p>Returns {@code true} if all conditions are met, indicating that sparse doc values | ||
| * should be used. Otherwise, returns {@code false}.</p> | ||
| * | ||
| * @param indexSortConfig The index sort configuration, used to check primary sorting. | ||
| * @param indexMode The mode of the index, which must be {@link IndexMode#LOGSDB}. | ||
| * @param fullFieldName The name of the field being checked, which must be {@code host.name}. | ||
| * @return {@code true} if sparse doc values should be used, otherwise {@code false}. | ||
| */ | ||
|
|
||
| private boolean shouldUseDocValuesSparseIndex( | ||
| private static boolean shouldUseDocValuesSkipper( | ||
| final boolean hasDocValues, | ||
| final IndexSortConfig indexSortConfig, | ||
| final IndexMode indexMode, | ||
| final String fullFieldName | ||
| ) { | ||
| if (indexed.isSet() && indexed.getValue()) { | ||
| return false; | ||
| } | ||
| return indexed.isConfigured() == false | ||
| && hasDocValues.getValue() | ||
| return hasDocValues | ||
| && IndexMode.LOGSDB.equals(indexMode) | ||
| && HOST_NAME.equals(fullFieldName) | ||
| && (indexSortConfig != null && indexSortConfig.hasPrimarySortOnField(HOST_NAME)); | ||
| && indexSortConfigByHostName(indexSortConfig); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here I assume we would like to create the sparse index on |
||
| } | ||
|
|
||
| private static boolean indexSortConfigByHostName(final IndexSortConfig indexSortConfig) { | ||
| return indexSortConfig != null && indexSortConfig.hasIndexSort() && indexSortConfig.hasSortOnFiled(HOST_NAME); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -492,7 +478,7 @@ public static final class KeywordFieldType extends StringFieldType { | |
| private final boolean isSyntheticSource; | ||
| private final IndexMode indexMode; | ||
| private final IndexSortConfig indexSortConfig; | ||
| private final boolean hasDocValuesSparseIndex; | ||
| private final boolean hasDocValuesSkipper; | ||
|
|
||
| public KeywordFieldType( | ||
| String name, | ||
|
|
@@ -520,7 +506,7 @@ public KeywordFieldType( | |
| this.isSyntheticSource = isSyntheticSource; | ||
| this.indexMode = builder.indexMode; | ||
| this.indexSortConfig = builder.indexSortConfig; | ||
| this.hasDocValuesSparseIndex = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false; | ||
| this.hasDocValuesSkipper = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false; | ||
| } | ||
|
|
||
| public KeywordFieldType(String name, boolean isIndexed, boolean hasDocValues, Map<String, String> meta) { | ||
|
|
@@ -534,7 +520,7 @@ public KeywordFieldType(String name, boolean isIndexed, boolean hasDocValues, Ma | |
| this.isSyntheticSource = false; | ||
| this.indexMode = IndexMode.STANDARD; | ||
| this.indexSortConfig = null; | ||
| this.hasDocValuesSparseIndex = false; | ||
| this.hasDocValuesSkipper = false; | ||
| } | ||
|
|
||
| public KeywordFieldType(String name) { | ||
|
|
@@ -559,7 +545,7 @@ public KeywordFieldType(String name, FieldType fieldType) { | |
| this.isSyntheticSource = false; | ||
| this.indexMode = IndexMode.STANDARD; | ||
| this.indexSortConfig = null; | ||
| this.hasDocValuesSparseIndex = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false; | ||
| this.hasDocValuesSkipper = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false; | ||
| } | ||
|
|
||
| public KeywordFieldType(String name, NamedAnalyzer analyzer) { | ||
|
|
@@ -573,7 +559,7 @@ public KeywordFieldType(String name, NamedAnalyzer analyzer) { | |
| this.isSyntheticSource = false; | ||
| this.indexMode = IndexMode.STANDARD; | ||
| this.indexSortConfig = null; | ||
| this.hasDocValuesSparseIndex = false; | ||
| this.hasDocValuesSkipper = false; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -1021,8 +1007,8 @@ public IndexSortConfig getIndexSortConfig() { | |
| return indexSortConfig; | ||
| } | ||
|
|
||
| public boolean hasDocValuesSparseIndex() { | ||
| return hasDocValuesSparseIndex; | ||
| public boolean hasDocValuesSkipper() { | ||
| return hasDocValuesSkipper; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1041,13 +1027,15 @@ public boolean hasDocValuesSparseIndex() { | |
| private final int ignoreAboveDefault; | ||
| private final IndexMode indexMode; | ||
| private final IndexSortConfig indexSortConfig; | ||
| private final boolean useDocValuesSkipper; | ||
|
|
||
| private KeywordFieldMapper( | ||
| String simpleName, | ||
| FieldType fieldType, | ||
| KeywordFieldType mappedFieldType, | ||
| BuilderParams builderParams, | ||
| boolean isSyntheticSource, | ||
| boolean useDocValuesSkipper, | ||
| Builder builder | ||
| ) { | ||
| super(simpleName, mappedFieldType, builderParams); | ||
|
|
@@ -1066,6 +1054,7 @@ private KeywordFieldMapper( | |
| this.ignoreAboveDefault = builder.ignoreAboveDefault; | ||
| this.indexMode = builder.indexMode; | ||
| this.indexSortConfig = builder.indexSortConfig; | ||
| this.useDocValuesSkipper = useDocValuesSkipper; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -1183,9 +1172,16 @@ public Map<String, NamedAnalyzer> indexAnalyzers() { | |
|
|
||
| @Override | ||
| public FieldMapper.Builder getMergeBuilder() { | ||
| return new Builder(leafName(), indexAnalyzers, scriptCompiler, ignoreAboveDefault, indexCreatedVersion, indexMode, indexSortConfig) | ||
| .dimension(fieldType().isDimension()) | ||
| .init(this); | ||
| return new Builder( | ||
| leafName(), | ||
| indexAnalyzers, | ||
| scriptCompiler, | ||
| ignoreAboveDefault, | ||
| indexCreatedVersion, | ||
| indexMode, | ||
| indexSortConfig, | ||
| useDocValuesSkipper | ||
| ).dimension(fieldType().isDimension()).init(this); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.