[9.0] Avoid serializing empty _source fields in mappings. (#124200)

martijnvg · web-flow · commit 359a8b3590f6 · 2025-03-07T20:55:04.000+01:00
Backporting #122606 to 9.0 branch.
diff --git a/docs/changelog/122606.yaml b/docs/changelog/122606.yaml
@@ -0,0 +1,5 @@
+pr: 122606
+summary: Avoid serializing empty `_source` fields in mappings
+area: Mapping
+type: bug
+issues: []
diff --git a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartDownsampleIT.java b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartDownsampleIT.java
@@ -44,16 +44,26 @@ public class FullClusterRestartDownsampleIT extends ParameterizedFullClusterRest
 
     protected static LocalClusterConfigProvider clusterConfig = c -> {};
 
-    private static ElasticsearchCluster cluster = ElasticsearchCluster.local()
-        .distribution(DistributionType.DEFAULT)
-        .version(Version.fromString(OLD_CLUSTER_VERSION))
-        .nodes(2)
-        .setting("xpack.security.enabled", "false")
-        .setting("indices.lifecycle.poll_interval", "5s")
-        .apply(() -> clusterConfig)
-        .feature(FeatureFlag.TIME_SERIES_MODE)
-        .feature(FeatureFlag.FAILURE_STORE_ENABLED)
-        .build();
+    private static ElasticsearchCluster cluster = buildCluster();
+
+    private static ElasticsearchCluster buildCluster() {
+        Version oldVersion = Version.fromString(OLD_CLUSTER_VERSION);
+        var cluster = ElasticsearchCluster.local()
+            .distribution(DistributionType.DEFAULT)
+            .version(Version.fromString(OLD_CLUSTER_VERSION))
+            .nodes(2)
+            .setting("xpack.security.enabled", "false")
+            .setting("indices.lifecycle.poll_interval", "5s")
+            .apply(() -> clusterConfig)
+            .feature(FeatureFlag.TIME_SERIES_MODE)
+            .feature(FeatureFlag.FAILURE_STORE_ENABLED);
+
+        if (oldVersion.before(Version.fromString("9.0.0"))) {
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
+        }
+        return cluster.build();
+    }
 
     @ClassRule
     public static TestRule ruleChain = RuleChain.outerRule(repoDirectory).around(cluster);
diff --git a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/FullClusterRestartIT.java
@@ -103,18 +103,28 @@ public class FullClusterRestartIT extends ParameterizedFullClusterRestartTestCas
 
     protected static LocalClusterConfigProvider clusterConfig = c -> {};
 
-    private static ElasticsearchCluster cluster = ElasticsearchCluster.local()
-        .distribution(DistributionType.DEFAULT)
-        .version(Version.fromString(OLD_CLUSTER_VERSION))
-        .nodes(2)
-        .setting("path.repo", () -> repoDirectory.getRoot().getPath())
-        .setting("xpack.security.enabled", "false")
-        // some tests rely on the translog not being flushed
-        .setting("indices.memory.shard_inactive_time", "60m")
-        .apply(() -> clusterConfig)
-        .feature(FeatureFlag.TIME_SERIES_MODE)
-        .feature(FeatureFlag.FAILURE_STORE_ENABLED)
-        .build();
+    private static ElasticsearchCluster cluster = buildCluster();
+
+    private static ElasticsearchCluster buildCluster() {
+        Version oldVersion = Version.fromString(OLD_CLUSTER_VERSION);
+        var cluster = ElasticsearchCluster.local()
+            .distribution(DistributionType.DEFAULT)
+            .version(Version.fromString(OLD_CLUSTER_VERSION))
+            .nodes(2)
+            .setting("path.repo", () -> repoDirectory.getRoot().getPath())
+            .setting("xpack.security.enabled", "false")
+            // some tests rely on the translog not being flushed
+            .setting("indices.memory.shard_inactive_time", "60m")
+            .apply(() -> clusterConfig)
+            .feature(FeatureFlag.TIME_SERIES_MODE)
+            .feature(FeatureFlag.FAILURE_STORE_ENABLED);
+
+        if (oldVersion.before(Version.fromString("9.0.0"))) {
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
+        }
+        return cluster.build();
+    }
 
     @ClassRule
     public static TestRule ruleChain = RuleChain.outerRule(repoDirectory).around(cluster);
diff --git a/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/LogsIndexModeFullClusterRestartIT.java b/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/upgrades/LogsIndexModeFullClusterRestartIT.java
@@ -33,17 +33,27 @@
 public class LogsIndexModeFullClusterRestartIT extends ParameterizedFullClusterRestartTestCase {
 
     @ClassRule
-    public static final ElasticsearchCluster cluster = ElasticsearchCluster.local()
-        .distribution(DistributionType.DEFAULT)
-        .version(Version.fromString(OLD_CLUSTER_VERSION))
-        .module("constant-keyword")
-        .module("data-streams")
-        .module("mapper-extras")
-        .module("x-pack-aggregate-metric")
-        .module("x-pack-stack")
-        .setting("xpack.security.enabled", "false")
-        .setting("xpack.license.self_generated.type", "trial")
-        .build();
+    public static final ElasticsearchCluster cluster = buildCluster();
+
+    private static ElasticsearchCluster buildCluster() {
+        Version oldVersion = Version.fromString(OLD_CLUSTER_VERSION);
+        var cluster = ElasticsearchCluster.local()
+            .distribution(DistributionType.DEFAULT)
+            .version(Version.fromString(OLD_CLUSTER_VERSION))
+            .module("constant-keyword")
+            .module("data-streams")
+            .module("mapper-extras")
+            .module("x-pack-aggregate-metric")
+            .module("x-pack-stack")
+            .setting("xpack.security.enabled", "false")
+            .setting("xpack.license.self_generated.type", "trial");
+
+        if (oldVersion.before(Version.fromString("9.0.0"))) {
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
+        }
+        return cluster.build();
+    }
 
     public LogsIndexModeFullClusterRestartIT(@Name("cluster") FullClusterRestartUpgradeStatus upgradeStatus) {
         super(upgradeStatus);
diff --git a/qa/mixed-cluster/build.gradle b/qa/mixed-cluster/build.gradle
@@ -91,6 +91,10 @@ buildParams.bwcVersions.withWireCompatible { bwcVersion, baseName ->
         setting 'health.master_history.no_master_transitions_threshold', '10'
       }
       requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0")
+      if (bwcVersion.before(Version.fromString("9.0.0"))) {
+        jvmArgs '-da:org.elasticsearch.index.mapper.DocumentMapper'
+        jvmArgs '-da:org.elasticsearch.index.mapper.MapperService'
+      }
     }
 
     tasks.register("${baseName}#mixedClusterTest", StandaloneRestIntegTestTask) {
diff --git a/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/AbstractRollingUpgradeTestCase.java b/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/AbstractRollingUpgradeTestCase.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.test.cluster.ElasticsearchCluster;
 import org.elasticsearch.test.cluster.FeatureFlag;
 import org.elasticsearch.test.cluster.local.distribution.DistributionType;
+import org.elasticsearch.test.cluster.util.Version;
 import org.junit.ClassRule;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TemporaryFolder;
@@ -26,20 +27,30 @@ public abstract class AbstractRollingUpgradeTestCase extends ParameterizedRollin
 
     private static final TemporaryFolder repoDirectory = new TemporaryFolder();
 
-    private static final ElasticsearchCluster cluster = ElasticsearchCluster.local()
-        .distribution(DistributionType.DEFAULT)
-        .version(getOldClusterTestVersion())
-        .nodes(NODE_NUM)
-        .setting("path.repo", new Supplier<>() {
-            @Override
-            @SuppressForbidden(reason = "TemporaryFolder only has io.File methods, not nio.File")
-            public String get() {
-                return repoDirectory.getRoot().getPath();
-            }
-        })
-        .setting("xpack.security.enabled", "false")
-        .feature(FeatureFlag.TIME_SERIES_MODE)
-        .build();
+    private static final ElasticsearchCluster cluster = buildCluster();
+
+    private static ElasticsearchCluster buildCluster() {
+        Version oldVersion = Version.fromString(OLD_CLUSTER_VERSION);
+        var cluster = ElasticsearchCluster.local()
+            .distribution(DistributionType.DEFAULT)
+            .version(getOldClusterTestVersion())
+            .nodes(NODE_NUM)
+            .setting("path.repo", new Supplier<>() {
+                @Override
+                @SuppressForbidden(reason = "TemporaryFolder only has io.File methods, not nio.File")
+                public String get() {
+                    return repoDirectory.getRoot().getPath();
+                }
+            })
+            .setting("xpack.security.enabled", "false")
+            .feature(FeatureFlag.TIME_SERIES_MODE);
+
+        if (oldVersion.before(Version.fromString("9.0.0"))) {
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
+        }
+        return cluster.build();
+    }
 
     @ClassRule
     public static TestRule ruleChain = RuleChain.outerRule(repoDirectory).around(cluster);
diff --git a/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/ParameterizedRollingUpgradeTestCase.java b/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/ParameterizedRollingUpgradeTestCase.java
@@ -36,7 +36,7 @@
 
 public abstract class ParameterizedRollingUpgradeTestCase extends ESRestTestCase {
     protected static final int NODE_NUM = 3;
-    private static final String OLD_CLUSTER_VERSION = System.getProperty("tests.old_cluster_version");
+    protected static final String OLD_CLUSTER_VERSION = System.getProperty("tests.old_cluster_version");
     private static final Set<Integer> upgradedNodes = new HashSet<>();
     private static TestFeatureService oldClusterTestFeatureService = null;
     private static boolean upgradeFailed = false;
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java
@@ -171,15 +171,17 @@ public Builder(
                 || settings.getAsBoolean(LOSSY_PARAMETERS_ALLOWED_SETTING_NAME, true);
             this.sourceModeIsNoop = sourceModeIsNoop;
             this.serializeMode = serializeMode;
-            this.mode = new Parameter<>(
-                "mode",
-                true,
-                () -> 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)),
-                v -> v.toString().toLowerCase(Locale.ROOT)
-            ).setMergeValidator((previous, current, conflicts) -> (previous == current) || current != Mode.STORED)
+            this.mode = new Parameter<>("mode", true, () -> null, (n, c, o) -> Mode.valueOf(o.toString().toUpperCase(Locale.ROOT)), m -> {
+                var sfm = toType(m);
+                if (sfm.enabled.explicit()) {
+                    return null;
+                } else if (sfm.serializeMode) {
+                    return sfm.mode;
+                } else {
+                    return null;
+                }
+            }, (b, n, v) -> b.field(n, v.toString().toLowerCase(Locale.ROOT)), v -> v.toString().toLowerCase(Locale.ROOT))
+                .setMergeValidator((previous, current, conflicts) -> (previous == current) || current != Mode.STORED)
                 // don't emit if `enabled` is configured
                 .setSerializerCheck((includeDefaults, isConfigured, value) -> serializeMode && value != null);
         }
@@ -300,10 +302,11 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) {
         if (indexMode == IndexMode.STANDARD && settingSourceMode == Mode.STORED) {
             return DEFAULT;
         }
+        SourceFieldMapper sourceFieldMapper;
         if (onOrAfterDeprecateModeVersion(c.indexVersionCreated())) {
-            return resolveStaticInstance(settingSourceMode);
+            sourceFieldMapper = resolveStaticInstance(settingSourceMode);
         } else {
-            return new SourceFieldMapper(
+            sourceFieldMapper = new SourceFieldMapper(
                 settingSourceMode,
                 Explicit.IMPLICIT_TRUE,
                 Strings.EMPTY_ARRAY,
@@ -312,6 +315,8 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) {
                 c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_MODE_ATTRIBUTE_NOOP)
             );
         }
+        indexMode.validateSourceFieldMapper(sourceFieldMapper);
+        return sourceFieldMapper;
     },
         c -> new Builder(
             c.getIndexSettings().getMode(),
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java
@@ -267,14 +267,14 @@ public void testSyntheticSourceInTimeSeries() throws IOException {
         });
         DocumentMapper mapper = createTimeSeriesModeDocumentMapper(mapping);
         assertTrue(mapper.sourceMapper().isSynthetic());
-        assertEquals("{\"_source\":{}}", mapper.sourceMapper().toString());
+        assertEquals("{}", mapper.sourceMapper().toString());
     }
 
     public void testSyntheticSourceWithLogsIndexMode() throws IOException {
         XContentBuilder mapping = fieldMapping(b -> { b.field("type", "keyword"); });
         DocumentMapper mapper = createLogsModeDocumentMapper(mapping);
         assertTrue(mapper.sourceMapper().isSynthetic());
-        assertEquals("{\"_source\":{}}", mapper.sourceMapper().toString());
+        assertEquals("{}", mapper.sourceMapper().toString());
     }
 
     public void testSupportsNonDefaultParameterValues() throws IOException {
diff --git a/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java b/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java
@@ -19,13 +19,23 @@
 public class MixedClusterDownsampleRestIT extends ESClientYamlSuiteTestCase {
 
     @ClassRule
-    public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
-        .distribution(DistributionType.DEFAULT)
-        .withNode(node -> node.version(getOldVersion()))
-        .withNode(node -> node.version(Version.CURRENT))
-        .setting("xpack.security.enabled", "false")
-        .setting("xpack.license.self_generated.type", "trial")
-        .build();
+    public static ElasticsearchCluster cluster = buildCluster();
+
+    private static ElasticsearchCluster buildCluster() {
+        Version oldVersion = getOldVersion();
+        var cluster = ElasticsearchCluster.local()
+            .distribution(DistributionType.DEFAULT)
+            .withNode(node -> node.version(getOldVersion()))
+            .withNode(node -> node.version(Version.CURRENT))
+            .setting("xpack.security.enabled", "false")
+            .setting("xpack.license.self_generated.type", "trial");
+
+        if (oldVersion.before(Version.fromString("9.0.0"))) {
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
+        }
+        return cluster.build();
+    }
 
     static Version getOldVersion() {
         return Version.fromString(System.getProperty("tests.old_cluster_version"));
diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java
@@ -14,15 +14,20 @@
 public class Clusters {
     public static ElasticsearchCluster mixedVersionCluster() {
         Version oldVersion = Version.fromString(System.getProperty("tests.old_cluster_version"));
-        return ElasticsearchCluster.local()
+        var cluster = ElasticsearchCluster.local()
             .distribution(DistributionType.DEFAULT)
             .withNode(node -> node.version(oldVersion))
             .withNode(node -> node.version(Version.CURRENT))
             .withNode(node -> node.version(oldVersion))
             .withNode(node -> node.version(Version.CURRENT))
             .setting("xpack.security.enabled", "false")
             .setting("xpack.license.self_generated.type", "trial")
-            .setting("cluster.routing.rebalance.enable", "none") // disable relocation until we have retry in ESQL
-            .build();
+            .setting("cluster.routing.rebalance.enable", "none"); // disable relocation until we have retry in ESQL;
+        if (oldVersion.before(Version.fromString("9.0.0"))) {
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
+            cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
+        }
+        return cluster.build();
     }
+
 }
diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java
@@ -52,10 +52,6 @@ public class LogsdbSnapshotRestoreIT extends ESRestTestCase {
         .setting("path.repo", () -> getRepoPath())
         .setting("xpack.security.enabled", "false")
         .setting("xpack.license.self_generated.type", "trial")
-        // TODO: remove when initializing / serializing default SourceFieldMapper instance have been fixed:
-        // (SFM's mode attribute often gets initialized, even when mode attribute isn't set)
-        .jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper")
-        .jvmArg("-da:org.elasticsearch.index.mapper.MapperService")
         .build();
 
     @ClassRule
diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle
@@ -66,6 +66,10 @@ buildParams.bwcVersions.withWireCompatible { bwcVersion, baseName ->
     setting 'xpack.security.transport.ssl.key', 'testnode.pem'
     setting 'xpack.security.transport.ssl.certificate', 'testnode.crt'
     keystore 'xpack.security.transport.ssl.secure_key_passphrase', 'testnode'
+    if (bwcVersion.before('9.0.0')) {
+      jvmArgs '-da:org.elasticsearch.index.mapper.MapperService'
+      jvmArgs '-da:org.elasticsearch.index.mapper.DocumentMapper'
+    }
 
     if (bwcVersion.onOrAfter('7.0.0')) {
       setting 'xpack.security.authc.realms.file.file1.order', '0'
diff --git a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/DataStreamsUpgradeIT.java b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/DataStreamsUpgradeIT.java
@@ -231,7 +231,7 @@ private void compareIndexMetadata(
                 Map<String, Object> oldIndexMetadata = oldIndicesMetadata.get(oldIndexName);
                 Map<String, Object> upgradedIndexMetadata = upgradedIndexEntry.getValue();
                 compareSettings(oldIndexMetadata, upgradedIndexMetadata);
-                assertThat("Mappings did not match", upgradedIndexMetadata.get("mappings"), equalTo(oldIndexMetadata.get("mappings")));
+                compareMappings((Map<?, ?>) oldIndexMetadata.get("mappings"), (Map<?, ?>) upgradedIndexMetadata.get("mappings"));
                 assertThat("ILM states did not match", upgradedIndexMetadata.get("ilm"), equalTo(oldIndexMetadata.get("ilm")));
                 if (oldIndexName.equals(oldWriteIndex) == false) { // the old write index will have been rolled over by upgrade
                     assertThat(
@@ -323,6 +323,21 @@ private void compareSettings(Map<String, Object> oldIndexMetadata, Map<String, O
         }
     }
 
+    private void compareMappings(Map<?, ?> oldMappings, Map<?, ?> upgradedMappings) {
+        boolean ignoreSource = Version.fromString(UPGRADE_FROM_VERSION).before(Version.V_9_0_0);
+        if (ignoreSource) {
+            Map<?, ?> doc = (Map<?, ?>) oldMappings.get("_doc");
+            if (doc != null) {
+                Map<?, ?> sourceEntry = (Map<?, ?>) doc.get("_source");
+                if (sourceEntry != null && sourceEntry.isEmpty()) {
+                    doc.remove("_source");
+                }
+                assert doc.containsKey("_source") == false;
+            }
+        }
+        assertThat("Mappings did not match", upgradedMappings, equalTo(oldMappings));
+    }
+
     @SuppressWarnings("unchecked")
     private Map<String, Object> getIndexSettingsFromIndexMetadata(Map<String, Object> indexMetadata) {
         return (Map<String, Object>) ((Map<String, Object>) indexMetadata.get("settings")).get("index");

Original file line number	Diff line number	Diff line change
`@@ -91,6 +91,10 @@ buildParams.bwcVersions.withWireCompatible { bwcVersion, baseName ->`
`91`	`91`	`setting 'health.master_history.no_master_transitions_threshold', '10'`
`92`	`92`	`}`
`93`	`93`	`requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0")`
	`94`	`+ if (bwcVersion.before(Version.fromString("9.0.0"))) {`
	`95`	`+ jvmArgs '-da:org.elasticsearch.index.mapper.DocumentMapper'`
	`96`	`+ jvmArgs '-da:org.elasticsearch.index.mapper.MapperService'`
	`97`	`+ }`
`94`	`98`	`}`
`95`	`99`
`96`	`100`	`tasks.register("${baseName}#mixedClusterTest", StandaloneRestIntegTestTask) {`