Skip to content

Doc values sparse index on _tsid fields #122699

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.

Sign up for GitHub

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
merged 4 commits into from
Feb 19, 2025
Merged

Doc values sparse index on _tsid fields #122699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
25749f4
Use doc values skipper for _tsid field
jordan-powers Feb 14, 2025
8a3ecc8
Address review comments
jordan-powers Feb 19, 2025
1305b0d
Merge remote-tracking branch 'upstream/main' into tsid-sparse-doc-values
jordan-powers Feb 19, 2025
90177e6
Merge remote-tracking branch 'upstream/main' into tsid-sparse-doc-values
jordan-powers Feb 19, 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
13 changes: 7 additions & 6 deletions server/src/main/java/org/elasticsearch/index/IndexMode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MappingLookup;
import org.elasticsearch.index.mapper.MappingParserContext;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.ProvidedIdFieldMapper;
import org.elasticsearch.index.mapper.RoutingFieldMapper;
Expand Down Expand Up @@ -90,7 +91,7 @@ public TimestampBounds getTimestampBound(IndexMetadata indexMetadata) {
}

@Override
public MetadataFieldMapper timeSeriesIdFieldMapper() {
public MetadataFieldMapper timeSeriesIdFieldMapper(MappingParserContext c) {
// non time-series indices must not have a TimeSeriesIdFieldMapper
return null;
}
Expand Down Expand Up @@ -187,8 +188,8 @@ private static String routingRequiredBad() {
}

@Override
public MetadataFieldMapper timeSeriesIdFieldMapper() {
return TimeSeriesIdFieldMapper.INSTANCE;
public MetadataFieldMapper timeSeriesIdFieldMapper(MappingParserContext c) {
return TimeSeriesIdFieldMapper.getInstance(c);
}

@Override
Expand Down Expand Up @@ -277,7 +278,7 @@ public TimestampBounds getTimestampBound(IndexMetadata indexMetadata) {
}

@Override
public MetadataFieldMapper timeSeriesIdFieldMapper() {
public MetadataFieldMapper timeSeriesIdFieldMapper(MappingParserContext c) {
// non time-series indices must not have a TimeSeriesIdFieldMapper
return null;
}
Expand Down Expand Up @@ -348,7 +349,7 @@ public TimestampBounds getTimestampBound(IndexMetadata indexMetadata) {
}

@Override
public MetadataFieldMapper timeSeriesIdFieldMapper() {
public MetadataFieldMapper timeSeriesIdFieldMapper(MappingParserContext c) {
// non time-series indices must not have a TimeSeriesIdFieldMapper
return null;
}
Expand Down Expand Up @@ -518,7 +519,7 @@ public String getName() {
* the _tsid field. The field mapper will be added to the list of the metadata
* field mappers for the index.
*/
public abstract MetadataFieldMapper timeSeriesIdFieldMapper();
public abstract MetadataFieldMapper timeSeriesIdFieldMapper(MappingParserContext c);

/**
* Return an instance of the {@link TimeSeriesRoutingHashFieldMapper} that generates
Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/org/elasticsearch/index/IndexVersions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ private static Version parseUnchecked(String version) {
public static final IndexVersion UPGRADE_TO_LUCENE_10_1_0 = def(9_009_0_00, Version.LUCENE_10_1_0);
public static final IndexVersion USE_SYNTHETIC_SOURCE_FOR_RECOVERY_BY_DEFAULT = def(9_010_00_0, Version.LUCENE_10_1_0);
public static final IndexVersion TIMESTAMP_DOC_VALUES_SPARSE_INDEX = def(9_011_0_00, Version.LUCENE_10_1_0);
public static final IndexVersion TIME_SERIES_ID_DOC_VALUES_SPARSE_INDEX = def(9_012_0_00, Version.LUCENE_10_1_0);
/*
* STOP! READ THIS FIRST! No, really,
* ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _
Expand Down
41 changes: 34 additions & 7 deletions server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,36 @@ public class TimeSeriesIdFieldMapper extends MetadataFieldMapper {
public static final String NAME = "_tsid";
public static final String CONTENT_TYPE = "_tsid";
public static final TimeSeriesIdFieldType FIELD_TYPE = new TimeSeriesIdFieldType();
public static final TimeSeriesIdFieldMapper INSTANCE = new TimeSeriesIdFieldMapper();
Copy link
Member

@martijnvg martijnvg Feb 19, 2025

Choose a reason for hiding this comment

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

The reason why this is a singleton constant is to reduce memory usage of MapperService. Meta field mappers that can be initialized as a constant are a constant. A node can have many MapperService instances (one per allocated index).

I think can just define two singleton instances here and then decide to use which in IndexMode#timeSeriesIdFieldMapper(...) based on index versions and whether the feature flag is enabled?


private static final TimeSeriesIdFieldMapper INSTANCE_WITHOUT_SKIPPER = new TimeSeriesIdFieldMapper(false);
private static final TimeSeriesIdFieldMapper INSTANCE_WITH_SKIPPER = new TimeSeriesIdFieldMapper(true);

public static TimeSeriesIdFieldMapper getInstance(boolean useDocValuesSkipper) {
if (useDocValuesSkipper) {
return INSTANCE_WITH_SKIPPER;
} else {
return INSTANCE_WITHOUT_SKIPPER;
}
}

public static TimeSeriesIdFieldMapper getInstance(MappingParserContext context) {
boolean useDocValuesSkipper = context.indexVersionCreated().onOrAfter(IndexVersions.TIME_SERIES_ID_DOC_VALUES_SPARSE_INDEX)
&& context.getIndexSettings().useDocValuesSkipper();
return TimeSeriesIdFieldMapper.getInstance(useDocValuesSkipper);
}

@Override
public FieldMapper.Builder getMergeBuilder() {
return new Builder().init(this);
return new Builder(this.useDocValuesSkipper).init(this);
}

public static class Builder extends MetadataFieldMapper.Builder {

protected Builder() {
private final boolean useDocValuesSkipper;

protected Builder(boolean useDocValuesSkipper) {
super(NAME);
this.useDocValuesSkipper = useDocValuesSkipper;
}

@Override
Expand All @@ -66,11 +85,11 @@ protected Parameter<?>[] getParameters() {

@Override
public TimeSeriesIdFieldMapper build() {
return INSTANCE;
return TimeSeriesIdFieldMapper.getInstance(useDocValuesSkipper);
}
}

public static final TypeParser PARSER = new FixedTypeParser(c -> c.getIndexSettings().getMode().timeSeriesIdFieldMapper());
public static final TypeParser PARSER = new FixedTypeParser(c -> c.getIndexSettings().getMode().timeSeriesIdFieldMapper(c));

public static final class TimeSeriesIdFieldType extends MappedFieldType {
private TimeSeriesIdFieldType() {
Expand Down Expand Up @@ -115,8 +134,11 @@ public Query termQuery(Object value, SearchExecutionContext context) {
}
}

private TimeSeriesIdFieldMapper() {
private final boolean useDocValuesSkipper;
Copy link
Member

@martijnvg martijnvg Feb 19, 2025

Choose a reason for hiding this comment

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

I think we can avoid passing down index version here. We can set useDocValuesSkipper based on the following expressions: indexVersion.onOrAfter(IndexVersions.TIME_SERIES_ID_DOC_VALUES_SPARSE_INDEX) && context.getIndexSettings().useDocValuesSkipper()?


private TimeSeriesIdFieldMapper(boolean useDocValuesSkipper) {
super(FIELD_TYPE);
this.useDocValuesSkipper = useDocValuesSkipper;
}

@Override
Expand All @@ -135,7 +157,12 @@ public void postParse(DocumentParserContext context) throws IOException {
} else {
timeSeriesId = routingPathFields.buildHash().toBytesRef();
}
context.doc().add(new SortedDocValuesField(fieldType().name(), timeSeriesId));

if (this.useDocValuesSkipper) {
context.doc().add(SortedDocValuesField.indexedField(fieldType().name(), timeSeriesId));
Copy link
Contributor Author

@jordan-powers jordan-powers Feb 18, 2025

Choose a reason for hiding this comment

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

SortedDocValuesField#indexedField creates a new SortedDocValuesField using a private static FieldType that has the skip index enabled. It's the same as discussed for SortedNumericDocValuesField in #122161 (comment)

} else {
context.doc().add(new SortedDocValuesField(fieldType().name(), timeSeriesId));
}

BytesRef uidEncoded = TsidExtractingIdFieldMapper.createField(
context,
Expand Down