Adjust index version for deprecating source mode (#117183)

There was a bug in the version-checking logic for emitting deprecation warnings for source.mode in mappings.
elastic · Nov 25, 2024 · 631345f · 631345f
1 parent 4e1807f
commit 631345f
Show file tree

Hide file tree

Showing 14 changed files with 133 additions and 24 deletions.
diff --git a/...c/test/java/org/elasticsearch/datastreams/mapper/DataStreamTimestampFieldMapperTests.java b/...c/test/java/org/elasticsearch/datastreams/mapper/DataStreamTimestampFieldMapperTests.java
@@ -48,7 +48,8 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
         checker.registerConflictCheck(
             "enabled",
             timestampMapping(true, b -> b.startObject("@timestamp").field("type", "date").endObject()),
-            timestampMapping(false, b -> b.startObject("@timestamp").field("type", "date").endObject())
+            timestampMapping(false, b -> b.startObject("@timestamp").field("type", "date").endObject()),
+            dm -> {}
         );
         checker.registerUpdateCheck(
             timestampMapping(false, b -> b.startObject("@timestamp").field("type", "date").endObject()),

diff --git a/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/IndexingIT.java b/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/IndexingIT.java
@@ -20,6 +20,7 @@
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.index.mapper.DateFieldMapper;
+import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.index.mapper.TimeSeriesIdFieldMapper;
 import org.elasticsearch.test.ListMatcher;
 import org.elasticsearch.xcontent.XContentBuilder;
@@ -417,9 +418,15 @@ public void testSyntheticSource() throws IOException {
         if (isOldCluster()) {
             Request createIndex = new Request("PUT", "/synthetic");
             XContentBuilder indexSpec = XContentBuilder.builder(XContentType.JSON.xContent()).startObject();
+            boolean useIndexSetting = SourceFieldMapper.onOrAfterDeprecateModeVersion(getOldClusterIndexVersion());
+            if (useIndexSetting) {
+                indexSpec.startObject("settings").field("index.mapping.source.mode", "synthetic").endObject();
+            }
             indexSpec.startObject("mappings");
             {
-                indexSpec.startObject("_source").field("mode", "synthetic").endObject();
+                if (useIndexSetting == false) {
+                    indexSpec.startObject("_source").field("mode", "synthetic").endObject();
+                }
                 indexSpec.startObject("properties").startObject("kwd").field("type", "keyword").endObject().endObject();
             }
             indexSpec.endObject();

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
@@ -26,6 +26,7 @@
 import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.index.IndexMode;
 import org.elasticsearch.index.IndexSettings;
+import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.index.query.QueryShardException;
 import org.elasticsearch.index.query.SearchExecutionContext;
@@ -297,7 +298,7 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) {
         if (indexMode == IndexMode.STANDARD && settingSourceMode == Mode.STORED) {
             return DEFAULT;
         }
-        if (c.indexVersionCreated().onOrAfter(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER)) {
+        if (onOrAfterDeprecateModeVersion(c.indexVersionCreated())) {
             return resolveStaticInstance(settingSourceMode);
         } else {
             return new SourceFieldMapper(settingSourceMode, Explicit.IMPLICIT_TRUE, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, true);
@@ -307,14 +308,14 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) {
             c.getIndexSettings().getMode(),
             c.getSettings(),
             c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK),
-            c.indexVersionCreated().before(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER)
+            onOrAfterDeprecateModeVersion(c.indexVersionCreated()) == false
         )
     ) {
         @Override
         public MetadataFieldMapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext)
             throws MapperParsingException {
             assert name.equals(SourceFieldMapper.NAME) : name;
-            if (parserContext.indexVersionCreated().after(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER) && node.containsKey("mode")) {
+            if (onOrAfterDeprecateModeVersion(parserContext.indexVersionCreated()) && node.containsKey("mode")) {
                 deprecationLogger.critical(DeprecationCategory.MAPPINGS, "mapping_source_mode", SourceFieldMapper.DEPRECATION_WARNING);
             }
             return super.parse(name, node, parserContext);
@@ -481,4 +482,10 @@ public boolean isDisabled() {
     public boolean isStored() {
         return mode == null || mode == Mode.STORED;
     }
+
+    public static boolean onOrAfterDeprecateModeVersion(IndexVersion version) {
+        return version.onOrAfter(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER);
+        // Adjust versions after backporting.
+        // || version.between(IndexVersions.BACKPORT_DEPRECATE_SOURCE_MODE_MAPPER, IndexVersions.UPGRADE_TO_LUCENE_10_0_0);
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserContextTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserContextTests.java
@@ -133,5 +133,6 @@ public void testCreateDynamicMapperBuilderContext() throws IOException {
         assertEquals(ObjectMapper.Defaults.DYNAMIC, resultFromParserContext.getDynamic());
         assertEquals(MapperService.MergeReason.MAPPING_UPDATE, resultFromParserContext.getMergeReason());
         assertFalse(resultFromParserContext.isInNestedContext());
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java
@@ -52,7 +52,8 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
         checker.registerConflictCheck(
             "enabled",
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("enabled", false).endObject()),
-            topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("enabled", true).endObject())
+            topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("enabled", true).endObject()),
+            dm -> {}
         );
         checker.registerUpdateCheck(
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("enabled", true).endObject()),
@@ -62,14 +63,18 @@ protected void registerParameters(ParameterChecker checker) throws IOException {
         checker.registerUpdateCheck(
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "stored").endObject()),
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "synthetic").endObject()),
-            dm -> assertTrue(dm.metadataMapper(SourceFieldMapper.class).isSynthetic())
+            dm -> {
+                assertTrue(dm.metadataMapper(SourceFieldMapper.class).isSynthetic());
+                assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
+            }
         );
         checker.registerConflictCheck("includes", b -> b.array("includes", "foo*"));
         checker.registerConflictCheck("excludes", b -> b.array("excludes", "foo*"));
         checker.registerConflictCheck(
             "mode",
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "synthetic").endObject()),
-            topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "stored").endObject())
+            topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "stored").endObject()),
+            dm -> assertWarnings(SourceFieldMapper.DEPRECATION_WARNING)
         );
     }
 
@@ -206,20 +211,22 @@ public void testSyntheticDisabledNotSupported() {
             )
         );
         assertThat(e.getMessage(), containsString("Cannot set both [mode] and [enabled] parameters"));
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
     }
 
     public void testSyntheticUpdates() throws Exception {
         MapperService mapperService = createMapperService("""
             { "_doc" : { "_source" : { "mode" : "synthetic" } } }
             """);
-
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         SourceFieldMapper mapper = mapperService.documentMapper().sourceMapper();
         assertTrue(mapper.enabled());
         assertTrue(mapper.isSynthetic());
 
         merge(mapperService, """
             { "_doc" : { "_source" : { "mode" : "synthetic" } } }
             """);
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         mapper = mapperService.documentMapper().sourceMapper();
         assertTrue(mapper.enabled());
         assertTrue(mapper.isSynthetic());
@@ -230,11 +237,15 @@ public void testSyntheticUpdates() throws Exception {
         Exception e = expectThrows(IllegalArgumentException.class, () -> merge(mapperService, """
             { "_doc" : { "_source" : { "mode" : "stored" } } }
             """));
+
         assertThat(e.getMessage(), containsString("Cannot update parameter [mode] from [synthetic] to [stored]"));
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
 
         merge(mapperService, """
             { "_doc" : { "_source" : { "mode" : "disabled" } } }
             """);
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
+
         mapper = mapperService.documentMapper().sourceMapper();
         assertFalse(mapper.enabled());
         assertFalse(mapper.isSynthetic());
@@ -270,6 +281,7 @@ public void testSupportsNonDefaultParameterValues() throws IOException {
                 topMapping(b -> b.startObject("_source").field("mode", randomBoolean() ? "synthetic" : "stored").endObject())
             ).documentMapper().sourceMapper();
             assertThat(sourceFieldMapper, notNullValue());
+            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         Exception e = expectThrows(
             MapperParsingException.class,
@@ -301,6 +313,8 @@ public void testSupportsNonDefaultParameterValues() throws IOException {
                 .documentMapper()
                 .sourceMapper()
         );
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
+
         assertThat(e.getMessage(), containsString("Parameter [mode=disabled] is not allowed in source"));
 
         e = expectThrows(
@@ -409,6 +423,7 @@ public void testRecoverySourceWithSyntheticSource() throws IOException {
             ParsedDocument doc = docMapper.parse(source(b -> { b.field("field1", "value1"); }));
             assertNotNull(doc.rootDoc().getField("_recovery_source"));
             assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"field1\":\"value1\"}")));
+            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         {
             Settings settings = Settings.builder().put(INDICES_RECOVERY_SOURCE_ENABLED_SETTING.getKey(), false).build();
@@ -419,6 +434,7 @@ public void testRecoverySourceWithSyntheticSource() throws IOException {
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source(b -> b.field("field1", "value1")));
             assertNull(doc.rootDoc().getField("_recovery_source"));
+            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
     }
 
@@ -613,6 +629,7 @@ public void testRecoverySourceWithLogsCustom() throws IOException {
             ParsedDocument doc = docMapper.parse(source(b -> { b.field("@timestamp", "2012-02-13"); }));
             assertNotNull(doc.rootDoc().getField("_recovery_source"));
             assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}")));
+            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         {
             Settings settings = Settings.builder()
@@ -623,6 +640,7 @@ public void testRecoverySourceWithLogsCustom() throws IOException {
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source(b -> b.field("@timestamp", "2012-02-13")));
             assertNull(doc.rootDoc().getField("_recovery_source"));
+            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
     }
 
@@ -691,6 +709,7 @@ public void testRecoverySourceWithTimeSeriesCustom() throws IOException {
                 doc.rootDoc().getField("_recovery_source").binaryValue(),
                 equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
             );
+            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         {
             Settings settings = Settings.builder()
@@ -704,6 +723,7 @@ public void testRecoverySourceWithTimeSeriesCustom() throws IOException {
                 source("123", b -> b.field("@timestamp", "2012-02-13").field("field", randomAlphaOfLength(5)), null)
             );
             assertNull(doc.rootDoc().getField("_recovery_source"));
+            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java b/server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java
@@ -21,6 +21,7 @@
 import org.elasticsearch.index.engine.VersionConflictEngineException;
 import org.elasticsearch.index.get.GetResult;
 import org.elasticsearch.index.mapper.RoutingFieldMapper;
+import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 import org.elasticsearch.xcontent.XContentType;
 
@@ -114,6 +115,7 @@ public void testGetFromTranslogWithSyntheticSource() throws IOException {
             "mode": "synthetic"
             """;
         runGetFromTranslogWithOptions(docToIndex, sourceOptions, expectedFetchedSource, "\"long\"", 7L, true);
+        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
     }
 
     public void testGetFromTranslogWithDenseVector() throws IOException {

diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MetadataMapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MetadataMapperTestCase.java
@@ -38,7 +38,7 @@ protected boolean isSupportedOn(IndexVersion version) {
 
     protected abstract void registerParameters(ParameterChecker checker) throws IOException;
 
-    private record ConflictCheck(XContentBuilder init, XContentBuilder update) {}
+    private record ConflictCheck(XContentBuilder init, XContentBuilder update, Consumer<DocumentMapper> check) {}
 
     private record UpdateCheck(XContentBuilder init, XContentBuilder update, Consumer<DocumentMapper> check) {}
 
@@ -58,7 +58,7 @@ public void registerConflictCheck(String param, CheckedConsumer<XContentBuilder,
                 b.startObject(fieldName());
                 update.accept(b);
                 b.endObject();
-            })));
+            }), d -> {}));
         }
 
         /**
@@ -68,8 +68,8 @@ public void registerConflictCheck(String param, CheckedConsumer<XContentBuilder,
          * @param init   the initial mapping
          * @param update the updated mapping
          */
-        public void registerConflictCheck(String param, XContentBuilder init, XContentBuilder update) {
-            conflictChecks.put(param, new ConflictCheck(init, update));
+        public void registerConflictCheck(String param, XContentBuilder init, XContentBuilder update, Consumer<DocumentMapper> check) {
+            conflictChecks.put(param, new ConflictCheck(init, update, check));
         }
 
         public void registerUpdateCheck(XContentBuilder init, XContentBuilder update, Consumer<DocumentMapper> check) {
@@ -95,6 +95,7 @@ public final void testUpdates() throws IOException {
                 e.getMessage(),
                 anyOf(containsString("Cannot update parameter [" + param + "]"), containsString("different [" + param + "]"))
             );
+            checker.conflictChecks.get(param).check.accept(mapperService.documentMapper());
         }
         for (UpdateCheck updateCheck : checker.updateChecks) {
             MapperService mapperService = createMapperService(updateCheck.init);

diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java
@@ -112,7 +112,6 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -1835,9 +1834,10 @@ public static CreateIndexResponse createIndex(RestClient client, String name, Se
 
         if (settings != null && settings.getAsBoolean(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), true) == false) {
             expectSoftDeletesWarning(request, name);
-        } else if (isSyntheticSourceConfiguredInMapping(mapping)) {
-            request.setOptions(expectVersionSpecificWarnings(v -> v.compatible(SourceFieldMapper.DEPRECATION_WARNING)));
-        }
+        } else if (isSyntheticSourceConfiguredInMapping(mapping)
+            && SourceFieldMapper.onOrAfterDeprecateModeVersion(minimumIndexVersion())) {
+                request.setOptions(expectVersionSpecificWarnings(v -> v.current(SourceFieldMapper.DEPRECATION_WARNING)));
+            }
         final Response response = client.performRequest(request);
         try (var parser = responseAsParser(response)) {
             return TestResponseParsers.parseCreateIndexResponse(parser);
@@ -1898,8 +1898,30 @@ protected static boolean isSyntheticSourceConfiguredInMapping(String mapping) {
         if (sourceMapper == null) {
             return false;
         }
-        Object mode = sourceMapper.get("mode");
-        return mode != null && mode.toString().toLowerCase(Locale.ROOT).equals("synthetic");
+        return sourceMapper.get("mode") != null;
+    }
+
+    @SuppressWarnings("unchecked")
+    protected static boolean isSyntheticSourceConfiguredInTemplate(String template) {
+        if (template == null) {
+            return false;
+        }
+        var values = XContentHelper.convertToMap(JsonXContent.jsonXContent, template, false);
+        for (Object value : values.values()) {
+            Map<String, Object> mappings = (Map<String, Object>) ((Map<String, Object>) value).get("mappings");
+            if (mappings == null) {
+                continue;
+            }
+            Map<String, Object> sourceMapper = (Map<String, Object>) mappings.get(SourceFieldMapper.NAME);
+            if (sourceMapper == null) {
+                continue;
+            }
+            Object mode = sourceMapper.get("mode");
+            if (mode != null) {
+                return true;
+            }
+        }
+        return false;
     }
 
     protected static Map<String, Object> getIndexSettings(String index) throws IOException {

diff --git a/...ugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java b/...ugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java
@@ -96,7 +96,8 @@ private DeprecationChecks() {}
         IndexDeprecationChecks::checkIndexDataPath,
         IndexDeprecationChecks::storeTypeSettingCheck,
         IndexDeprecationChecks::frozenIndexSettingCheck,
-        IndexDeprecationChecks::deprecatedCamelCasePattern
+        IndexDeprecationChecks::deprecatedCamelCasePattern,
+        IndexDeprecationChecks::checkSourceModeInMapping
     );
 
     static List<BiFunction<DataStream, ClusterState, DeprecationIssue>> DATA_STREAM_CHECKS = List.of(

diff --git a/...deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecks.java b/...deprecation/src/main/java/org/elasticsearch/xpack/deprecation/IndexDeprecationChecks.java
@@ -16,6 +16,7 @@
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.index.engine.frozen.FrozenEngine;
+import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
 
 import java.util.ArrayList;
@@ -201,6 +202,31 @@ static List<String> findInPropertiesRecursively(
         return issues;
     }
 
+    static DeprecationIssue checkSourceModeInMapping(IndexMetadata indexMetadata, ClusterState clusterState) {
+        if (SourceFieldMapper.onOrAfterDeprecateModeVersion(indexMetadata.getCreationVersion())) {
+            boolean[] useSourceMode = { false };
+            fieldLevelMappingIssue(indexMetadata, ((mappingMetadata, sourceAsMap) -> {
+                Object source = sourceAsMap.get("_source");
+                if (source instanceof Map<?, ?> sourceMap) {
+                    if (sourceMap.containsKey("mode")) {
+                        useSourceMode[0] = true;
+                    }
+                }
+            }));
+            if (useSourceMode[0]) {
+                return new DeprecationIssue(
+                    DeprecationIssue.Level.CRITICAL,
+                    SourceFieldMapper.DEPRECATION_WARNING,
+                    "https://github.com/elastic/elasticsearch/pull/117172",
+                    SourceFieldMapper.DEPRECATION_WARNING,
+                    false,
+                    null
+                );
+            }
+        }
+        return null;
+    }
+
     static DeprecationIssue deprecatedCamelCasePattern(IndexMetadata indexMetadata, ClusterState clusterState) {
         List<String> fields = new ArrayList<>();
         fieldLevelMappingIssue(

diff --git a/...k/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/AsyncOperator.java b/...k/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/AsyncOperator.java
@@ -43,6 +43,7 @@ public abstract class AsyncOperator implements Operator {
 
     private final int maxOutstandingRequests;
     private final LongAdder totalTimeInNanos = new LongAdder();
+
     private boolean finished = false;
     private volatile boolean closed = false;