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

[8.x] Avoid serializing empty _source fields in mappings. #124199

Merged
merged 4 commits into from
Mar 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions docs/changelog/122606.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 122606
summary: Avoid serializing empty `_source` fields in mappings
area: Mapping
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import com.carrotsearch.randomizedtesting.annotations.Name;

import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -44,16 +45,26 @@ public class FullClusterRestartDownsampleIT extends ParameterizedFullClusterRest

protected static LocalClusterConfigProvider clusterConfig = c -> {};

private static ElasticsearchCluster cluster = ElasticsearchCluster.local()
.distribution(DistributionType.DEFAULT)
.version(getOldClusterTestVersion())
.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(getOldClusterTestVersion())
.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("8.19.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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import org.apache.http.util.EntityUtils;
import org.elasticsearch.Build;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.cluster.settings.RestClusterGetSettingsResponse;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
Expand Down Expand Up @@ -108,18 +109,28 @@ public class FullClusterRestartIT extends ParameterizedFullClusterRestartTestCas

protected static LocalClusterConfigProvider clusterConfig = c -> {};

private static ElasticsearchCluster cluster = ElasticsearchCluster.local()
.distribution(DistributionType.DEFAULT)
.version(getOldClusterTestVersion())
.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(getOldClusterTestVersion())
.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("8.19.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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import com.carrotsearch.randomizedtesting.annotations.Name;

import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.RestClient;
Expand All @@ -33,17 +34,27 @@
public class LogsIndexModeFullClusterRestartIT extends ParameterizedFullClusterRestartTestCase {

@ClassRule
public static final ElasticsearchCluster cluster = ElasticsearchCluster.local()
.distribution(DistributionType.DEFAULT)
.version(getOldClusterTestVersion())
.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(getOldClusterTestVersion())
.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("8.19.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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
@TestCaseOrdering(FullClusterRestartTestOrdering.class)
public abstract class ParameterizedFullClusterRestartTestCase extends ESRestTestCase {
private static final Version MINIMUM_WIRE_COMPATIBLE_VERSION = Version.fromString("7.17.0");
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 IndexVersion oldIndexVersion;
private static boolean upgradeFailed = false;
private static boolean upgraded = false;
Expand Down
4 changes: 4 additions & 0 deletions qa/mixed-cluster/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,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("8.19.0"))) {
jvmArgs '-da:org.elasticsearch.index.mapper.DocumentMapper'
jvmArgs '-da:org.elasticsearch.index.mapper.MapperService'
}
}

tasks.register("${baseName}#mixedClusterTest", StandaloneRestIntegTestTask) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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("8.19.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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,18 @@ public Builder(IndexMode indexMode, final Settings settings, boolean supportsChe
this.supportsNonDefaultParameterValues = supportsCheckForNonDefaultParams == false
|| settings.getAsBoolean(LOSSY_PARAMETERS_ALLOWED_SETTING_NAME, true);
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,
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)
v -> v != null ? v.toString().toLowerCase(Locale.ROOT) : null
).setMergeValidator((previous, current, conflicts) -> (previous == current) || current != Mode.STORED)
// don't emit if `enabled` is configured
.setSerializerCheck((includeDefaults, isConfigured, value) -> serializeMode && value != null);
Expand Down Expand Up @@ -300,11 +304,20 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) {
if (indexMode == IndexMode.STANDARD && settingSourceMode == Mode.STORED) {
return DEFAULT;
}
SourceFieldMapper sourceFieldMapper;
if (c.indexVersionCreated().onOrAfter(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER)) {
return resolveStaticInstance(settingSourceMode);
sourceFieldMapper = resolveStaticInstance(settingSourceMode);
} else {
return new SourceFieldMapper(settingSourceMode, Explicit.IMPLICIT_TRUE, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, true);
sourceFieldMapper = new SourceFieldMapper(
settingSourceMode,
Explicit.IMPLICIT_TRUE,
Strings.EMPTY_ARRAY,
Strings.EMPTY_ARRAY,
true
);
}
indexMode.validateSourceFieldMapper(sourceFieldMapper);
return sourceFieldMapper;
},
c -> new Builder(
c.getIndexSettings().getMode(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("8.19.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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ public static ElasticsearchCluster mixedVersionCluster() {
if (supportRetryOnShardFailures(oldVersion) == false) {
cluster.setting("cluster.routing.rebalance.enable", "none");
}
if (oldVersion.before(Version.fromString("8.19.0"))) {
cluster.jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper");
cluster.jvmArg("-da:org.elasticsearch.index.mapper.MapperService");
}
return cluster.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ public class LogsdbSnapshotRestoreIT extends ESRestTestCase {
.setting("cluster.logsdb.enabled", "true")
.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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ modify logsdb index source mode to stored after index creation:
_source:
mode: stored
- match: { error.type: "illegal_argument_exception" }
- match: { error.reason: "Mapper for [_source] conflicts with existing mapper:\n\tCannot update parameter [mode] from [synthetic] to [stored]" }
- match: { error.reason: "Mapper for [_source] conflicts with existing mapper:\n\tCannot update parameter [mode] from [null] to [stored]" }

---
modify time_series index source mode to disabled after index creation:
Expand Down Expand Up @@ -812,4 +812,4 @@ modify time_series index source mode to stored after index creation:
_source:
mode: stored
- match: { error.type: "illegal_argument_exception" }
- match: { error.reason: "Mapper for [_source] conflicts with existing mapper:\n\tCannot update parameter [mode] from [synthetic] to [stored]" }
- match: { error.reason: "Mapper for [_source] conflicts with existing mapper:\n\tCannot update parameter [mode] from [null] to [stored]" }
4 changes: 4 additions & 0 deletions x-pack/qa/rolling-upgrade/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,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('8.19.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'
Expand Down
Loading