Refactoring doc values sparse index enabling for the host.name field (#121751)

salvatore-campagna · web-flow · commit 27adf205edc6 · 2025-02-10T14:20:25.000+01:00
In this PR, we change how the doc values sparse index is enabled for the host.name keyword field. The initial implementation of the sparse index for host.name was introduced in #120741. Previously, the choice between using an inverted index or a doc values sparse index was determined by the index parameter. With this change, we introduce a new final index-level setting, index.mapping.use_doc_values_sparse_index: - When the setting is true, we enable the sparse index and omit the inverted index for host.name. - When the setting is false (default), we retain the inverted index instead. Additionally, this setting is only exposed if the doc_values_sparse_index feature flag is enabled. This change simplifies enabling the doc values sparse index and makes the selection of indexing strategies explicit at the index level. Moreover, the setting is not dynamic and is exposed only for stateful deployments. The plan is to enable this setting in our nightly benchmarks and evaluate its impact on LogsDB indexing throughput, storage footprint and query latency. Based on benchmarking results, we will decide whether to adopt the sparse index and determine the best way to configure it.
diff --git a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java
diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java
@@ -685,6 +685,14 @@ public boolean isES87TSDBCodecEnabled() {
         Property.Final
     );
 
+    public static final FeatureFlag DOC_VALUES_SKIPPER = new FeatureFlag("doc_values_skipper");
+    public static final Setting<Boolean> USE_DOC_VALUES_SKIPPER = Setting.boolSetting(
+        "index.mapping.use_doc_values_skipper",
+        IndexSettings.DOC_VALUES_SKIPPER.isEnabled(),
+        Property.IndexScope,
+        Property.Final
+    );
+
     /**
      * The {@link IndexMode "mode"} of the index.
      */
@@ -922,6 +930,7 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) {
     private final SourceFieldMapper.Mode indexMappingSourceMode;
     private final boolean recoverySourceEnabled;
     private final boolean recoverySourceSyntheticEnabled;
+    private final boolean useDocValuesSkipper;
 
     /**
      * The maximum number of refresh listeners allows on this shard.
@@ -1103,6 +1112,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
         recoverySourceEnabled = RecoverySettings.INDICES_RECOVERY_SOURCE_ENABLED_SETTING.get(nodeSettings);
         recoverySourceSyntheticEnabled = DiscoveryNode.isStateless(nodeSettings) == false
             && scopedSettings.get(RECOVERY_USE_SYNTHETIC_SOURCE_SETTING);
+        useDocValuesSkipper = DOC_VALUES_SKIPPER.isEnabled() && scopedSettings.get(USE_DOC_VALUES_SKIPPER);
         if (recoverySourceSyntheticEnabled) {
             if (DiscoveryNode.isStateless(settings)) {
                 throw new IllegalArgumentException("synthetic recovery source is only allowed in stateful");
@@ -1822,6 +1832,10 @@ public boolean isRecoverySourceSyntheticEnabled() {
         return recoverySourceSyntheticEnabled;
     }
 
+    public boolean useDocValuesSkipper() {
+        return useDocValuesSkipper;
+    }
+
     /**
      * The bounds for {@code @timestamp} on this index or
      * {@code null} if there are no bounds.
diff --git a/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java b/server/src/main/java/org/elasticsearch/index/IndexSortConfig.java
@@ -302,6 +302,15 @@ private static void validateIndexSortField(SortField sortField) {
         }
     }
 
+    public boolean hasSortOnField(final String fieldName) {
+        for (FieldSortSpec sortSpec : sortSpecs) {
+            if (sortSpec.field.equals(fieldName)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     public static class FieldSortSpec {
         final String field;
         SortOrder order;
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -21,7 +21,6 @@
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.util.CollectionUtils;
-import org.elasticsearch.common.util.FeatureFlag;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.IndexMode;
@@ -63,8 +62,6 @@
 
 public abstract class FieldMapper extends Mapper {
     private static final Logger logger = LogManager.getLogger(FieldMapper.class);
-
-    public static final FeatureFlag DOC_VALUES_SPARSE_INDEX = new FeatureFlag("doc_values_sparse_index");
     public static final Setting<Boolean> IGNORE_MALFORMED_SETTING = Setting.boolSetting("index.mapping.ignore_malformed", settings -> {
         if (IndexSettings.MODE.get(settings) == IndexMode.LOGSDB
             && IndexMetadata.SETTING_INDEX_VERSION_CREATED.get(settings).onOrAfter(IndexVersions.ENABLE_IGNORE_MALFORMED_LOGSDB)) {
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java
@@ -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);
+        }
+
+        private static boolean indexSortConfigByHostName(final IndexSortConfig indexSortConfig) {
+            return indexSortConfig != null && indexSortConfig.hasIndexSort() && indexSortConfig.hasSortOnField(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
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java
