- 
                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 21 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.