Skip to content
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

Set default index mode for TimeSeries to null #98808

Merged
merged 26 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d8c6ff6
Skip segment for MatchNoDocsQuery filters.
kkrik-es Aug 8, 2023
18fe70e
Update docs/changelog/98295.yaml
kkrik-es Aug 8, 2023
b4c15cc
Check all filters for `MatchNoDocsQuery`.
kkrik-es Aug 9, 2023
0d74960
Merge remote-tracking branch 'upstream/fix/94637' into fix/94637
kkrik-es Aug 9, 2023
3d0d573
Merge branch 'main' into fix/94637
kkrik-es Aug 10, 2023
d4e6563
Skip optimization when 'other' bucket is requested.
kkrik-es Aug 10, 2023
0034e96
Merge branch 'main' into fix/94637
kkrik-es Aug 11, 2023
1a939cb
Merge branch 'main' into fix/94637
kkrik-es Aug 23, 2023
d22c37f
Revert "Set default index mode for TimeSeries to `null` (#98586)"
kkrik-es Aug 23, 2023
ac5859d
Merge branch 'main' into fix/94637
kkrik-es Aug 23, 2023
1b0eaf3
Revert "Rollback of #98586 (#98805)"
kkrik-es Aug 23, 2023
a11a7d2
Skip updating source when missing synthetic mode
kkrik-es Aug 23, 2023
653d0a3
Update docs/changelog/98808.yaml
kkrik-es Aug 23, 2023
0b97ea5
Merge branch 'main' into fix/94637
kkrik-es Aug 24, 2023
2dc9f1b
Merge remote-tracking branch 'upstream/fix/94637' into fix/94637
kkrik-es Aug 24, 2023
1829f68
Skip matching assert in MapperService too
kkrik-es Aug 24, 2023
392c8fc
Refine the assert
kkrik-es Aug 24, 2023
b37628d
Extend versions before 8.6, when TS had no synthetic source
kkrik-es Aug 24, 2023
e759e58
Add source field mapping for non-synthetic TSDB
kkrik-es Aug 24, 2023
cc703c3
Delete 98586.yaml
kkrik-es Aug 24, 2023
e582cda
Add comment to TSDB_NO_SYNTHETIC mapping
kkrik-es Aug 24, 2023
bb6696e
Merge branch 'main' into fix/94637
kkrik-es Aug 24, 2023
0f9d987
Merge remote-tracking branch 'upstream/fix/94637' into fix/94637
kkrik-es Aug 24, 2023
9b96f7b
Spotless fix
kkrik-es Aug 24, 2023
3ac901d
Add yaml test
kkrik-es Aug 25, 2023
0fa3956
Fix version skip in yaml test
kkrik-es Aug 25, 2023
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
6 changes: 6 additions & 0 deletions docs/changelog/98808.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 98808
summary: Set default index mode for `TimeSeries` to `null`
area: Aggregations
type: enhancement
issues:
- 97429
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.IndexVersion;

import java.util.List;

Expand All @@ -30,18 +31,30 @@ public static DocumentMapper createEmpty(MapperService mapperService) {
);
MetadataFieldMapper[] metadata = mapperService.getMetadataMappers().values().toArray(new MetadataFieldMapper[0]);
Mapping mapping = new Mapping(root, metadata, null);
return new DocumentMapper(mapperService.documentParser(), mapping, mapping.toCompressedXContent());
return new DocumentMapper(mapperService.documentParser(), mapping, mapping.toCompressedXContent(), IndexVersion.current());
}

DocumentMapper(DocumentParser documentParser, Mapping mapping, CompressedXContent source) {
DocumentMapper(DocumentParser documentParser, Mapping mapping, CompressedXContent source, IndexVersion version) {
this.documentParser = documentParser;
this.type = mapping.getRoot().name();
this.mappingLookup = MappingLookup.fromMapping(mapping);
this.mappingSource = source;
assert mapping.toCompressedXContent().equals(source)

assert mapping.toCompressedXContent().equals(source) || isSyntheticSourceMalformed(source, version)
: "provided source [" + source + "] differs from mapping [" + mapping.toCompressedXContent() + "]";
}

/**
* Indexes built at v.8.7 were missing an explicit entry for synthetic_source.
* This got restored in v.8.10 to avoid confusion. The change is only restricted to mapping printout, it has no
* functional effect as the synthetic source already applies.
*/
boolean isSyntheticSourceMalformed(CompressedXContent source, IndexVersion version) {
return sourceMapper().isSynthetic()
&& source.string().contains("\"_source\":{\"mode\":\"synthetic\"}") == false
&& version.onOrBefore(IndexVersion.V_8_10_0);
}

public Mapping mapping() {
return mappingLookup.getMapping();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,13 +348,15 @@ boolean assertNoUpdateRequired(final IndexMetadata newIndexMetadata) {
Mapping newMapping = parseMapping(mapping.type(), mapping.source());
final CompressedXContent currentSource = this.mapper.mappingSource();
final CompressedXContent newSource = newMapping.toCompressedXContent();
if (Objects.equals(currentSource, newSource) == false) {
if (Objects.equals(currentSource, newSource) == false
&& mapper.isSyntheticSourceMalformed(currentSource, indexVersionCreated) == false) {
throw new IllegalStateException(
"expected current mapping [" + currentSource + "] to be the same as new mapping [" + newSource + "]"
);
}
}
return true;

}

public void merge(IndexMetadata indexMetadata, MergeReason reason) {
Expand Down Expand Up @@ -386,7 +388,7 @@ public DocumentMapper merge(String type, CompressedXContent mappingSource, Merge
}

private DocumentMapper newDocumentMapper(Mapping mapping, MergeReason reason, CompressedXContent mappingSource) {
DocumentMapper newMapper = new DocumentMapper(documentParser, mapping, mappingSource);
DocumentMapper newMapper = new DocumentMapper(documentParser, mapping, mappingSource, indexVersionCreated);
newMapper.validate(indexSettings, reason != MergeReason.MAPPING_RECOVERY);
return newMapper;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.IndexMode;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.search.lookup.Source;
Expand Down Expand Up @@ -61,6 +62,18 @@ private enum Mode {
IndexMode.TIME_SERIES
);

/*
* Synthetic source was added as the default for TSDB in v.8.7. The legacy field mapper below
* is used in bwc tests and mixed clusters containing time series indexes created in an earlier version.
*/
private static final SourceFieldMapper TSDB_NO_SYNTHETIC = new SourceFieldMapper(
kkrik-es marked this conversation as resolved.
Show resolved Hide resolved
null,
Explicit.IMPLICIT_TRUE,
Strings.EMPTY_ARRAY,
Strings.EMPTY_ARRAY,
IndexMode.TIME_SERIES
);

public static class Defaults {
public static final String NAME = SourceFieldMapper.NAME;

Expand All @@ -86,10 +99,15 @@ public static class Builder extends MetadataFieldMapper.Builder {
.setMergeValidator(
(previous, current, conflicts) -> (previous.value() == current.value()) || (previous.value() && current.value() == false)
);

/*
* The default mode for TimeSeries is left empty on purpose, so that mapping printings include the synthetic
* source mode.
*/
private final Parameter<Mode> mode = new Parameter<>(
"mode",
true,
() -> getIndexMode() == IndexMode.TIME_SERIES ? Mode.SYNTHETIC : null,
() -> null,
(n, c, o) -> Mode.valueOf(o.toString().toUpperCase(Locale.ROOT)),
m -> toType(m).enabled.explicit() ? null : toType(m).mode,
(b, n, v) -> b.field(n, v.toString().toLowerCase(Locale.ROOT)),
Expand Down Expand Up @@ -136,10 +154,11 @@ private boolean isDefault() {

@Override
public SourceFieldMapper build() {
if (enabled.getValue().explicit() && mode.get() != null) {
if (enabled.getValue().explicit()) {
if (indexMode == IndexMode.TIME_SERIES) {
throw new MapperParsingException("Time series indices only support synthetic source");
} else {
}
if (mode.get() != null) {
throw new MapperParsingException("Cannot set both [mode] and [enabled] parameters");
}
}
Expand All @@ -165,7 +184,9 @@ private IndexMode getIndexMode() {
}

public static final TypeParser PARSER = new ConfigurableTypeParser(
c -> c.getIndexSettings().getMode() == IndexMode.TIME_SERIES ? TSDB_DEFAULT : DEFAULT,
c -> c.getIndexSettings().getMode() == IndexMode.TIME_SERIES
? c.getIndexSettings().getIndexVersionCreated().onOrAfter(IndexVersion.V_8_7_0) ? TSDB_DEFAULT : TSDB_NO_SYNTHETIC
: DEFAULT,
c -> new Builder(c.getIndexSettings().getMode())
);

Expand Down Expand Up @@ -217,7 +238,7 @@ private SourceFieldMapper(Mode mode, Explicit<Boolean> enabled, String[] include
this.sourceFilter = buildSourceFilter(includes, excludes);
this.includes = includes;
this.excludes = excludes;
if (this.sourceFilter != null && mode == Mode.SYNTHETIC) {
if (this.sourceFilter != null && (mode == Mode.SYNTHETIC || indexMode == IndexMode.TIME_SERIES)) {
throw new IllegalArgumentException("filtering the stored _source is incompatible with synthetic source");
}
this.complete = stored() && sourceFilter == null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void testAddFields() throws Exception {
assertThat(stage1.mappers().getMapper("obj1.prop1"), nullValue());
// but merged should
DocumentParser documentParser = new DocumentParser(null, null, () -> DocumentParsingObserver.EMPTY_INSTANCE);
DocumentMapper mergedMapper = new DocumentMapper(documentParser, merged, merged.toCompressedXContent());
DocumentMapper mergedMapper = new DocumentMapper(documentParser, merged, merged.toCompressedXContent(), IndexVersion.current());
assertThat(mergedMapper.mappers().getMapper("age"), notNullValue());
assertThat(mergedMapper.mappers().getMapper("obj1.prop1"), notNullValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.plugins.MapperPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.script.CompositeFieldScript;
Expand Down Expand Up @@ -2530,7 +2531,12 @@ same name need to be part of the same mappings (hence the same document). If th

// merge without going through toXContent and reparsing, otherwise the potential leaf path issue gets fixed on its own
Mapping newMapping = MapperService.mergeMappings(mapperService.documentMapper(), mapping, MapperService.MergeReason.MAPPING_UPDATE);
DocumentMapper newDocMapper = new DocumentMapper(mapperService.documentParser(), newMapping, newMapping.toCompressedXContent());
DocumentMapper newDocMapper = new DocumentMapper(
mapperService.documentParser(),
newMapping,
newMapping.toCompressedXContent(),
IndexVersion.current()
);
ParsedDocument doc2 = newDocMapper.parse(source("""
{
"foo" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentFactory;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentType;
Expand Down Expand Up @@ -227,4 +228,14 @@ public void testSyntheticUpdates() throws Exception {
assertFalse(mapper.enabled());
assertFalse(mapper.isSynthetic());
}

public void testSyntheticSourceInTimeSeries() throws IOException {
XContentBuilder mapping = fieldMapping(b -> {
b.field("type", "keyword");
b.field("time_series_dimension", true);
});
DocumentMapper mapper = createTimeSeriesModeDocumentMapper(mapping);
assertTrue(mapper.sourceMapper().isSynthetic());
assertEquals("{\"_source\":{\"mode\":\"synthetic\"}}", mapper.sourceMapper().toString());
}
}