Allow stored source in logsdb and tsdb (#114454)

lkts · web-flow · commit a62228a74422 · 2024-10-11T11:16:55.000-07:00
diff --git a/modules/aggregations/build.gradle b/modules/aggregations/build.gradle
@@ -48,4 +48,5 @@ dependencies {
 
 tasks.named("yamlRestCompatTestTransform").configure({ task ->
   task.skipTest("aggregations/date_agg_per_day_of_week/Date aggregartion per day of week", "week-date behaviour has changed")
+  task.skipTest("aggregations/time_series/Configure with no synthetic source", "temporary until backport")
 })
diff --git a/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/time_series.yml b/modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/time_series.yml
@@ -291,23 +291,6 @@ setup:
                     sum:
                       sum:
                         field: val
----
-"Configure with no synthetic source":
-  - requires:
-      cluster_features: ["gte_v8.15.0"]
-      reason: "Error message changed in 8.15.0"
-
-  - do:
-      catch: '/Indices with with index mode \[time_series\] only support synthetic source/'
-      indices.create:
-        index: tsdb_error
-        body:
-          settings:
-            mode: time_series
-            routing_path: [key]
-          mappings:
-            _source:
-              enabled: false
 
 ---
 "Number for keyword routing field":
diff --git a/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java b/modules/data-streams/src/javaRestTest/java/org/elasticsearch/datastreams/logsdb/LogsIndexModeCustomSettingsIT.java
@@ -15,7 +15,6 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
 import org.elasticsearch.test.cluster.local.distribution.DistributionType;
-import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.ClassRule;
 
@@ -115,18 +114,62 @@ public void testConfigureStoredSourceBeforeIndexCreation() throws IOException {
               }
             }""";
 
+        assertOK(putComponentTemplate(client, "logs@custom", storedSourceMapping));
+        assertOK(createDataStream(client, "logs-custom-dev"));
+
+        var mapping = getMapping(client, getDataStreamBackingIndex(client, "logs-custom-dev", 0));
+        String sourceMode = (String) subObject("_source").apply(mapping).get("mode");
+        assertThat(sourceMode, equalTo("stored"));
+    }
+
+    public void testConfigureDisabledSourceBeforeIndexCreation() {
+        var storedSourceMapping = """
+            {
+              "template": {
+                "settings": {
+                  "index": {
+                    "mode": "logsdb"
+                  }
+                },
+                "mappings": {
+                  "_source": {
+                    "enabled": false
+                  }
+                }
+              }
+            }""";
+
         Exception e = assertThrows(ResponseException.class, () -> putComponentTemplate(client, "logs@custom", storedSourceMapping));
         assertThat(
             e.getMessage(),
-            containsString("Failed to parse mapping: Indices with with index mode [logsdb] only support synthetic source")
+            containsString("Failed to parse mapping: _source can not be disabled in index using [logsdb] index mode")
         );
         assertThat(e.getMessage(), containsString("mapper_parsing_exception"));
+    }
 
-        assertOK(createDataStream(client, "logs-custom-dev"));
+    public void testConfigureDisabledSourceModeBeforeIndexCreation() {
+        var storedSourceMapping = """
+            {
+              "template": {
+                "settings": {
+                  "index": {
+                    "mode": "logsdb"
+                  }
+                },
+                "mappings": {
+                  "_source": {
+                    "mode": "disabled"
+                  }
+                }
+              }
+            }""";
 
-        var mapping = getMapping(client, getDataStreamBackingIndex(client, "logs-custom-dev", 0));
-        String sourceMode = (String) subObject("_source").apply(mapping).get("mode");
-        assertThat(sourceMode, equalTo("synthetic"));
+        Exception e = assertThrows(ResponseException.class, () -> putComponentTemplate(client, "logs@custom", storedSourceMapping));
+        assertThat(
+            e.getMessage(),
+            containsString("Failed to parse mapping: _source can not be disabled in index using [logsdb] index mode")
+        );
+        assertThat(e.getMessage(), containsString("mapper_parsing_exception"));
     }
 
     public void testConfigureStoredSourceWhenIndexIsCreated() throws IOException {
@@ -142,8 +185,45 @@ public void testConfigureStoredSourceWhenIndexIsCreated() throws IOException {
             }""";
 
         assertOK(putComponentTemplate(client, "logs@custom", storedSourceMapping));
+        assertOK(createDataStream(client, "logs-custom-dev"));
+
+        var mapping = getMapping(client, getDataStreamBackingIndex(client, "logs-custom-dev", 0));
+        String sourceMode = (String) subObject("_source").apply(mapping).get("mode");
+        assertThat(sourceMode, equalTo("stored"));
+    }
+
+    public void testConfigureDisabledSourceWhenIndexIsCreated() throws IOException {
+        var disabledModeMapping = """
+            {
+              "template": {
+                "mappings": {
+                  "_source": {
+                    "enabled": false
+                  }
+                }
+              }
+            }""";
+
+        assertOK(putComponentTemplate(client, "logs@custom", disabledModeMapping));
+        ResponseException e = expectThrows(ResponseException.class, () -> createDataStream(client, "logs-custom-dev"));
+        assertThat(e.getMessage(), containsString("_source can not be disabled in index using [logsdb] index mode"));
+    }
+
+    public void testConfigureDisabledSourceModeWhenIndexIsCreated() throws IOException {
+        var disabledModeMapping = """
+            {
+              "template": {
+                "mappings": {
+                  "_source": {
+                    "mode": "disabled"
+                  }
+                }
+              }
+            }""";
+
+        assertOK(putComponentTemplate(client, "logs@custom", disabledModeMapping));
         ResponseException e = expectThrows(ResponseException.class, () -> createDataStream(client, "logs-custom-dev"));
-        assertThat(e.getMessage(), containsString("Indices with with index mode [logsdb] only support synthetic source"));
+        assertThat(e.getMessage(), containsString("_source can not be disabled in index using [logsdb] index mode"));
     }
 
     public void testOverrideIndexCodec() throws IOException {
@@ -377,7 +457,7 @@ public void testIgnoreAboveSetting() throws IOException {
                 );
                 assertThat(
                     ex.getMessage(),
-                    Matchers.containsString("Failed to parse value [" + newValue + "] for setting [index.mapping.ignore_above]")
+                    containsString("Failed to parse value [" + newValue + "] for setting [index.mapping.ignore_above]")
                 );
             }
         }
diff --git a/rest-api-spec/build.gradle b/rest-api-spec/build.gradle
@@ -57,4 +57,8 @@ tasks.named("precommit").configure {
 tasks.named("yamlRestCompatTestTransform").configure({task ->
     task.skipTest("indices.sort/10_basic/Index Sort", "warning does not exist for compatibility")
     task.skipTest("search/330_fetch_fields/Test search rewrite", "warning does not exist for compatibility")
+    task.skipTest("tsdb/20_mapping/disabled source", "temporary until backported")
+    task.skipTest("logsdb/20_source_mapping/disabled _source is not supported", "temporary until backported")
+    task.skipTest("tsdb/20_mapping/regular source", "temporary until backported")
+    task.skipTest("logsdb/20_source_mapping/stored _source mode is not supported", "temporary until backported")
 })
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/logsdb/20_source_mapping.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/logsdb/20_source_mapping.yml
@@ -1,21 +1,10 @@
 ---
-stored _source mode is not supported:
+stored _source mode is supported:
   - requires:
-      test_runner_features: [capabilities]
-      capabilities:
-        - method: PUT
-          path: /{index}
-          capabilities: [logsdb_index_mode]
-      reason: "Support for 'logsdb' index mode capability required"
-
-  - skip:
-      known_issues:
-        - cluster_feature: "gte_v8.15.0"
-          fixed_by: "gte_v8.16.0"
-      reason: "Development of logs index mode spans 8.15 and 8.16"
+      cluster_features: ["mapper.source.remove_synthetic_source_only_validation"]
+      reason: requires new validation logic
 
   - do:
-      catch: bad_request
       indices.create:
         index: test-stored-source
         body:
@@ -25,31 +14,17 @@ stored _source mode is not supported:
           mappings:
             _source:
               mode: stored
-            properties:
-              "@timestamp":
-                type: date
-              host.name:
-                type: keyword
+  - do:
+      indices.get:
+        index: test-stored-source
 
-  - match: { error.type: "mapper_parsing_exception" }
-  - match: { error.root_cause.0.type: "mapper_parsing_exception" }
-  - match: { error.reason: "Failed to parse mapping: Indices with with index mode [logsdb] only support synthetic source" }
+  - match: { test-stored-source.mappings._source.mode: "stored" }
 
 ---
 disabled _source is not supported:
   - requires:
-      test_runner_features: [capabilities]
-      capabilities:
-        - method: PUT
-          path: /{index}
-          capabilities: [logsdb_index_mode]
-      reason: "Support for 'logsdb' index mode capability required"
-
-  - skip:
-      known_issues:
-        - cluster_feature: "gte_v8.15.0"
-          fixed_by: "gte_v8.16.0"
-      reason: "Development of logs index mode spans 8.15 and 8.16"
+      cluster_features: ["mapper.source.remove_synthetic_source_only_validation"]
+      reason: requires new error message
 
   - do:
       catch: bad_request
@@ -62,33 +37,23 @@ disabled _source is not supported:
           mappings:
             _source:
               enabled: false
-            properties:
-              "@timestamp":
-                type: date
-              host.name:
-                type: keyword
 
   - match: { error.type: "mapper_parsing_exception" }
   - match: { error.root_cause.0.type: "mapper_parsing_exception" }
-  - match: { error.reason: "Failed to parse mapping: Indices with with index mode [logsdb] only support synthetic source" }
+  - match: { error.reason: "Failed to parse mapping: _source can not be disabled in index using [logsdb] index mode" }
 
   - do:
       catch: bad_request
       indices.create:
-        index: test-disabled-source
+        index: test-disabled-mode-source
         body:
           settings:
             index:
               mode: logsdb
           mappings:
             _source:
               mode: disabled
-            properties:
-              "@timestamp":
-                type: date
-              host.name:
-                type: keyword
 
   - match: { error.type: "mapper_parsing_exception" }
   - match: { error.root_cause.0.type: "mapper_parsing_exception" }
-  - match: { error.reason: "Failed to parse mapping: Indices with with index mode [logsdb] only support synthetic source" }
+  - match: { error.reason: "Failed to parse mapping: _source can not be disabled in index using [logsdb] index mode" }
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mapping.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/20_mapping.yml
@@ -456,13 +456,12 @@ nested fields:
   - match: {tsdb-synthetic.mappings._source.mode: synthetic}
 
 ---
-regular source:
+stored source is supported:
   - requires:
-      cluster_features: ["gte_v8.7.0"]
-      reason: synthetic source
+      cluster_features: ["mapper.source.remove_synthetic_source_only_validation"]
+      reason: requires new validation logic
 
   - do:
-      catch: '/time series indices only support synthetic source/'
       indices.create:
         index: tsdb_index
         body:
@@ -486,14 +485,21 @@ regular source:
                       uid:
                         type: keyword
                         time_series_dimension: true
+
+  - do:
+      indices.get:
+        index: tsdb_index
+
+  - match: { tsdb_index.mappings._source.mode: "stored" }
+
 ---
-disabled source:
+disabled source is not supported:
   - requires:
-      cluster_features: ["gte_v8.7.0"]
-      reason: synthetic source
+      cluster_features: ["mapper.source.remove_synthetic_source_only_validation"]
+      reason: requires new error message
 
   - do:
-      catch: '/time series indices only support synthetic source/'
+      catch: bad_request
       indices.create:
         index: tsdb_index
         body:
@@ -518,6 +524,10 @@ disabled source:
                         type: keyword
                         time_series_dimension: true
 
+  - match: { error.type: "mapper_parsing_exception" }
+  - match: { error.root_cause.0.type: "mapper_parsing_exception" }
+  - match: { error.reason: "Failed to parse mapping: _source can not be disabled in index using [time_series] index mode" }
+
 ---
 source include/exclude:
   - requires:
diff --git a/server/src/main/java/org/elasticsearch/index/IndexMode.java b/server/src/main/java/org/elasticsearch/index/IndexMode.java
@@ -217,8 +217,8 @@ public boolean shouldValidateTimestamp() {
 
         @Override
         public void validateSourceFieldMapper(SourceFieldMapper sourceFieldMapper) {
-            if (sourceFieldMapper.isSynthetic() == false) {
-                throw new IllegalArgumentException("time series indices only support synthetic source");
+            if (sourceFieldMapper.enabled() == false) {
+                throw new IllegalArgumentException("_source can not be disabled in index using [" + IndexMode.TIME_SERIES + "] index mode");
             }
         }
 
@@ -292,8 +292,8 @@ public boolean shouldValidateTimestamp() {
 
         @Override
         public void validateSourceFieldMapper(SourceFieldMapper sourceFieldMapper) {
-            if (sourceFieldMapper.isSynthetic() == false) {
-                throw new IllegalArgumentException("Indices with with index mode [" + IndexMode.LOGSDB + "] only support synthetic source");
+            if (sourceFieldMapper.enabled() == false) {
+                throw new IllegalArgumentException("_source can not be disabled in index using [" + IndexMode.LOGSDB + "] index mode");
             }
         }
 
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java
@@ -58,6 +58,10 @@ public Set<NodeFeature> getFeatures() {
 
     @Override
     public Set<NodeFeature> getTestFeatures() {
-        return Set.of(RangeFieldMapper.DATE_RANGE_INDEXING_FIX, IgnoredSourceFieldMapper.DONT_EXPAND_DOTS_IN_IGNORED_SOURCE);
+        return Set.of(
+            RangeFieldMapper.DATE_RANGE_INDEXING_FIX,
+            IgnoredSourceFieldMapper.DONT_EXPAND_DOTS_IN_IGNORED_SOURCE,
+            SourceFieldMapper.REMOVE_SYNTHETIC_SOURCE_ONLY_VALIDATION
+        );
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java
@@ -12,7 +12,6 @@
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
-import org.elasticsearch.index.IndexMode;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.xcontent.XContentType;
 
@@ -147,10 +146,6 @@ Mapping parse(@Nullable String type, MergeReason reason, Map<String, Object> map
                 assert fieldNodeMap.isEmpty();
 
                 if (metadataFieldMapper instanceof SourceFieldMapper sfm) {
-                    // Validation in other places should have failed first
-                    assert sfm.isSynthetic()
-                        || (sfm.isSynthetic() == false && mappingParserContext.getIndexSettings().getMode() != IndexMode.TIME_SERIES)
-                        : "synthetic source can't be disabled in a time series index";
                     isSourceSynthetic = sfm.isSynthetic();
                 }
 
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
@@ -51,6 +51,9 @@ public class SourceFieldMapper extends MetadataFieldMapper {
     public static final NodeFeature SYNTHETIC_SOURCE_COPY_TO_INSIDE_OBJECTS_FIX = new NodeFeature(
         "mapper.source.synthetic_source_copy_to_inside_objects_fix"
     );
+    public static final NodeFeature REMOVE_SYNTHETIC_SOURCE_ONLY_VALIDATION = new NodeFeature(
+        "mapper.source.remove_synthetic_source_only_validation"
+    );
 
     public static final String NAME = "_source";
     public static final String RECOVERY_SOURCE_NAME = "_recovery_source";
@@ -235,9 +238,6 @@ private boolean isDefault() {
         @Override
         public SourceFieldMapper build() {
             if (enabled.getValue().explicit()) {
-                if (indexMode != null && indexMode.isSyntheticSourceEnabled()) {
-                    throw new MapperParsingException("Indices with with index mode [" + indexMode + "] only support synthetic source");
-                }
                 if (mode.get() != null) {
                     throw new MapperParsingException("Cannot set both [mode] and [enabled] parameters");
                 }

Original file line number	Diff line number	Diff line change
`@@ -217,8 +217,8 @@ public boolean shouldValidateTimestamp() {`
`217`	`217`
`218`	`218`	`@Override`
`219`	`219`	`public void validateSourceFieldMapper(SourceFieldMapper sourceFieldMapper) {`
`220`		`- if (sourceFieldMapper.isSynthetic() == false) {`
`221`		`- throw new IllegalArgumentException("time series indices only support synthetic source");`
	`220`	`+ if (sourceFieldMapper.enabled() == false) {`
	`221`	`+ throw new IllegalArgumentException("_source can not be disabled in index using [" + IndexMode.TIME_SERIES + "] index mode");`
`222`	`222`	`}`
`223`	`223`	`}`
`224`	`224`
`@@ -292,8 +292,8 @@ public boolean shouldValidateTimestamp() {`
`292`	`292`
`293`	`293`	`@Override`
`294`	`294`	`public void validateSourceFieldMapper(SourceFieldMapper sourceFieldMapper) {`
`295`		`- if (sourceFieldMapper.isSynthetic() == false) {`
`296`		`- throw new IllegalArgumentException("Indices with with index mode [" + IndexMode.LOGSDB + "] only support synthetic source");`
	`295`	`+ if (sourceFieldMapper.enabled() == false) {`
	`296`	`+ throw new IllegalArgumentException("_source can not be disabled in index using [" + IndexMode.LOGSDB + "] index mode");`
`297`	`297`	`}`
`298`	`298`	`}`
`299`	`299`
Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,10 @@ public Set<NodeFeature> getFeatures() {`
`58`	`58`
`59`	`59`	`@Override`
`60`	`60`	`public Set<NodeFeature> getTestFeatures() {`
`61`		`- return Set.of(RangeFieldMapper.DATE_RANGE_INDEXING_FIX, IgnoredSourceFieldMapper.DONT_EXPAND_DOTS_IN_IGNORED_SOURCE);`
	`61`	`+ return Set.of(`
	`62`	`+ RangeFieldMapper.DATE_RANGE_INDEXING_FIX,`
	`63`	`+ IgnoredSourceFieldMapper.DONT_EXPAND_DOTS_IN_IGNORED_SOURCE,`
	`64`	`+ SourceFieldMapper.REMOVE_SYNTHETIC_SOURCE_ONLY_VALIDATION`
	`65`	`+ );`
`62`	`66`	`}`
`63`	`67`	`}`