Avoid serializing empty _source fields in mappings. (#122606)

This commit is contained in:
Martijn van Groningen 2025-03-06 12:20:07 +01:00 committed by GitHub
parent 387eef070c
commit ea8283e9c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 157 additions and 73 deletions

View file

@ -0,0 +1,5 @@
pr: 122606
summary: Avoid serializing empty `_source` fields in mappings
area: Mapping
type: bug
issues: []

View file

@ -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.1.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);

View file

@ -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.1.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);

View file

@ -33,17 +33,27 @@ import java.util.function.Supplier;
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.1.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);

View file

@ -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.1.0"))) {
jvmArgs '-da:org.elasticsearch.index.mapper.DocumentMapper'
jvmArgs '-da:org.elasticsearch.index.mapper.MapperService'
}
}
tasks.register("${baseName}#mixedClusterTest", StandaloneRestIntegTestTask) {

View file

@ -15,6 +15,7 @@ import org.elasticsearch.core.SuppressForbidden;
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.1.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);

View file

@ -36,7 +36,7 @@ import static org.hamcrest.Matchers.notNullValue;
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;

View file

@ -171,15 +171,17 @@ public class SourceFieldMapper extends MetadataFieldMapper {
|| 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 @@ public class SourceFieldMapper extends MetadataFieldMapper {
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 @@ public class SourceFieldMapper extends MetadataFieldMapper {
c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_MODE_ATTRIBUTE_NOOP)
);
}
indexMode.validateSourceFieldMapper(sourceFieldMapper);
return sourceFieldMapper;
},
c -> new Builder(
c.getIndexSettings().getMode(),

View file

@ -267,14 +267,14 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
});
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 {

View file

@ -19,13 +19,23 @@ import org.junit.ClassRule;
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.1.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"));

View file

@ -25,6 +25,10 @@ public class Clusters {
if (supportRetryOnShardFailures(oldVersion) == false) {
cluster.setting("cluster.routing.rebalance.enable", "none");
}
if (oldVersion.before(Version.fromString("9.1.0"))) {
cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
}
return cluster.build();
}

View file

@ -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

View file

@ -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.1.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'

View file

@ -217,7 +217,7 @@ public class DataStreamsUpgradeIT extends AbstractUpgradeTestCase {
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(
@ -268,6 +268,21 @@ public class DataStreamsUpgradeIT extends AbstractUpgradeTestCase {
}
}
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");