Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 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 Feb 5, 2025
c8e6ce2
fix: refactor check on sorting and remove index param check
salvatore-campagna Feb 5, 2025
fee1b5a
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 5, 2025
7b795ba
fix: relay on default setting value
salvatore-campagna Feb 5, 2025
115e30c
fix: simplify sort check on host.name
salvatore-campagna Feb 5, 2025
598776d
fix: rework logic to conditionally add the new setting
salvatore-campagna Feb 5, 2025
a9b76ce
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 5, 2025
efad1cd
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 5, 2025
85f00a4
fix: make methods static and update javadoc
salvatore-campagna Feb 5, 2025
1317e2d
docs: remove javadocs, method is self-explanatory
salvatore-campagna Feb 5, 2025
0f760e3
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 5, 2025
0a40c32
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 5, 2025
2214c97
fix: default true for snapshot builds, false otherwise
salvatore-campagna Feb 5, 2025
7afba8e
fix: gate usage of sparse index with feature flag
salvatore-campagna Feb 6, 2025
531a3e9
fix: udpate test according to default value
salvatore-campagna Feb 6, 2025
066a1d3
test: run both in snapshot and release builds
salvatore-campagna Feb 6, 2025
3015eb2
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 6, 2025
eda9755
not: rename doc values sparse index to doc values skipper
salvatore-campagna Feb 7, 2025
41c6098
fix: delete file
salvatore-campagna Feb 7, 2025
30a0c13
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 7, 2025
0fc3f4b
Merge branch 'main' into feature/hostname-doc-values-sparse-index-set…
salvatore-campagna Feb 10, 2025
9ab1a9a
fix: typo 'Filed' instead of 'Field'
salvatore-campagna Feb 10, 2025
8a405f2
fix: replace assertEquals with if statement
salvatore-campagna Feb 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,14 @@ public boolean isES87TSDBCodecEnabled() {
Property.Final
);

public static final FeatureFlag DOC_VALUES_SPARSE_INDEX = new FeatureFlag("doc_values_sparse_index");
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 moved the feature flag here since we now use it to enable the setting or not.

Copy link
Member

Choose a reason for hiding this comment

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

Keep the setting here, but move the setting registration to logsdb plugin?

public static final Setting<Boolean> USE_DOC_VALUES_SPARSE_INDEX = Setting.boolSetting(
"index.mapping.use_doc_values_sparse_index",
Copy link
Member

Choose a reason for hiding this comment

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

Let's use the lucene name. So use_doc_values_skipper?

Choose a reason for hiding this comment

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

Note that in IndexVersions I think I cannot rename public static final IndexVersion HOSTNAME_DOC_VALUES_SPARSE_INDEX = def(9_008_0_00, Version.LUCENE_10_0_0);
I will rename everything else.

IndexSettings.DOC_VALUES_SPARSE_INDEX.isEnabled(),
Property.IndexScope,
Property.Final
);

/**
* The {@link IndexMode "mode"} of the index.
*/
Expand Down Expand Up @@ -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 useDocValuesSparseIndex;

/**
* The maximum number of refresh listeners allows on this shard.
Expand Down Expand Up @@ -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);
useDocValuesSparseIndex = DOC_VALUES_SPARSE_INDEX.isEnabled() && scopedSettings.get(USE_DOC_VALUES_SPARSE_INDEX);
if (recoverySourceSyntheticEnabled) {
if (DiscoveryNode.isStateless(settings)) {
throw new IllegalArgumentException("synthetic recovery source is only allowed in stateful");
Expand Down Expand Up @@ -1822,6 +1832,10 @@ public boolean isRecoverySourceSyntheticEnabled() {
return recoverySourceSyntheticEnabled;
}

public boolean useDocValuesSparseIndex() {
return useDocValuesSparseIndex;
}

/**
* The bounds for {@code @timestamp} on this index or
* {@code null} if there are no bounds.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,15 @@ private static void validateIndexSortField(SortField sortField) {
}
}

public boolean hasSortOnFiled(final String fieldName) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public boolean hasSortOnFiled(final String fieldName) {
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_SPARSE_INDEX;

/**
* A field mapper for keywords. This mapper accepts strings and indexes them as-is.
Expand Down Expand Up @@ -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 useDocValuesSparseIndex;

public Builder(final String name, final MappingParserContext mappingParserContext) {
this(
Expand All @@ -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_SPARSE_INDEX.get(mappingParserContext.getSettings())
);
}

Expand All @@ -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(
Expand All @@ -231,7 +234,8 @@ private Builder(
int ignoreAboveDefault,
IndexVersion indexCreatedVersion,
IndexMode indexMode,
IndexSortConfig indexSortConfig
IndexSortConfig indexSortConfig,
boolean useDocValuesSparseIndex
) {
super(name);
this.indexAnalyzers = indexAnalyzers;
Expand Down Expand Up @@ -268,6 +272,7 @@ private Builder(
});
this.indexSortConfig = indexSortConfig;
this.indexMode = indexMode;
this.useDocValuesSparseIndex = useDocValuesSparseIndex;
}

public Builder(String name, IndexVersion indexCreatedVersion) {
Expand Down Expand Up @@ -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(
useDocValuesSparseIndex,
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);
Expand All @@ -417,65 +428,40 @@ public KeywordFieldMapper build(MapperBuilderContext context) {
buildFieldType(context, fieldtype),
builderParams(this, context),
context.isSourceSynthetic(),
useDocValuesSparseIndex,
this
);
}

private FieldType resolveFieldType(
final boolean useDocValuesSparseIndex,
final IndexVersion indexCreatedVersion,
final IndexSortConfig indexSortConfig,
final IndexMode indexMode,
final String fullFieldName
) {
if (FieldMapper.DOC_VALUES_SPARSE_INDEX.isEnabled()
if (useDocValuesSparseIndex
&& indexCreatedVersion.onOrAfter(IndexVersions.HOSTNAME_DOC_VALUES_SPARSE_INDEX)
&& shouldUseDocValuesSparseIndex(indexSortConfig, indexMode, fullFieldName)) {
&& shouldUseDocValuesSparseIndex(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 shouldUseDocValuesSparseIndex(
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);
Copy link
Contributor Author

@salvatore-campagna salvatore-campagna Feb 6, 2025

Choose a reason for hiding this comment

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

Here I assume we would like to create the sparse index on host.name no matter if it is the first field or not in the index sort configuration.

}

private static boolean indexSortConfigByHostName(final IndexSortConfig indexSortConfig) {
return indexSortConfig != null && indexSortConfig.hasIndexSort() && indexSortConfig.hasSortOnFiled(HOST_NAME);
}
}

Expand Down Expand Up @@ -1041,13 +1027,15 @@ public boolean hasDocValuesSparseIndex() {
private final int ignoreAboveDefault;
private final IndexMode indexMode;
private final IndexSortConfig indexSortConfig;
private final boolean useDocValuesSParseIndex;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe rename to: useDocValuesSkipper ?


private KeywordFieldMapper(
String simpleName,
FieldType fieldType,
KeywordFieldType mappedFieldType,
BuilderParams builderParams,
boolean isSyntheticSource,
boolean useDocValuesSParseIndex,
Builder builder
) {
super(simpleName, mappedFieldType, builderParams);
Expand All @@ -1066,6 +1054,7 @@ private KeywordFieldMapper(
this.ignoreAboveDefault = builder.ignoreAboveDefault;
this.indexMode = builder.indexMode;
this.indexSortConfig = builder.indexSortConfig;
this.useDocValuesSParseIndex = useDocValuesSParseIndex;
}

@Override
Expand Down Expand Up @@ -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,
useDocValuesSParseIndex
).dimension(fieldType().isDimension()).init(this);
}

@Override
Expand Down
Loading