Skip to content

Commit

Permalink
Revert "Deprecate _source.mode in mappings (#117106)" (#117151)
Browse files Browse the repository at this point in the history
This reverts #117106. Bwc tests fail, because older nodes are killed with the following error:

```
[2024-11-20T10:54:58,600][ERROR][o.e.b.ElasticsearchUncaughtExceptionHandler] [v8.17.0-0] fatal error in thread [elasticsearch[v8.17.0-0
][clusterApplierService#updateTask][T#1]], exiting java.lang.AssertionError: provided source [{"_doc":{"_data_stream_timestamp":{"enabled":true},"_source":{},"properties":{"@timestamp":{"type":"date"},"k8s":{"properties":{"pod":{"properties":{"ip":{"type":"ip"},"name":{"type":"keyword"},"network":{"properties":{"rx":{"type":"long"},"tx":{"type":"long"}}},"uid":{"type":"keyword","time_series_dimension":true}}}}},"metricset":{"type":"keyword","time_series_dimension":true}}}}] differs from mapping [{"_doc":{"_data_stream_timestamp":{"enabled":true},"_source":{"mode":"synthetic"},"properties":{"@timestamp":{"type":"date"},"k8s":{"properties":{"pod":{"properties":{"ip":{"type":"ip"},"name":{"type":"keyword"},"network":{"properties":{"rx":{"type":"long"},"tx":{"type":"long"}}},"uid":{"type":"keyword","time_series_dimension":true}}}}},"metricset":{"type":"keyword","time_series_dimension":true}}}}]
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.index.mapper.DocumentMapper.<init>(DocumentMapper.java:66)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.index.mapper.MapperService.newDocumentMapper(MapperService.java:588)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.index.mapper.MapperService.updateMapping(MapperService.java:346)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.index.IndexService.updateMapping(IndexService.java:840)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.indices.cluster.IndicesClusterStateService.createIndicesAndUpdateShards(IndicesClusterStateService.java:583)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.indices.cluster.IndicesClusterStateService.doApplyClusterState(IndicesClusterStateService.java:306)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.indices.cluster.IndicesClusterStateService.applyClusterState(IndicesClusterStateService.java:260)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.cluster.service.ClusterApplierService.callClusterStateAppliers(ClusterApplierService.java:544)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.cluster.service.ClusterApplierService.callClusterStateAppliers(ClusterApplierService.java:530)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.cluster.service.ClusterApplierService.applyChanges(ClusterApplierService.java:503)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.cluster.service.ClusterApplierService.runTask(ClusterApplierService.java:432)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.cluster.service.ClusterApplierService$UpdateTask.run(ClusterApplierService.java:157)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:956)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedEsThreadPoolExecutor.java:218)
        at org.elasticsearch.server@9.0.0-SNAPSHOT/org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:184)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
        at java.base/java.lang.Thread.run(Thread.java:1575)
```

The `mode` parameter no longer gets serialized for new indices. However on the older nodes still serialize the `mode` parameter, which caused the menioned assertion to fail. Reverting for now and see how best to address this bwc serialization issue.

We can only stop serializing mode, when all nodes are on the same version.  Unfortunately we can't invoke `c.clusterTransportVersion().get()` from parser or builder, because that calling thread isn't allowed to call `clusterService.state()`.
  • Loading branch information
martijnvg authored Nov 20, 2024
1 parent d03917d commit 1bc60ac
Show file tree
Hide file tree
Showing 23 changed files with 133 additions and 170 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ public void skipTest(String fullTestName, String reason) {
// For example: indices.get_mapping/20_missing_type/Non-existent type returns 404
// However, the folder can be arbitrarily nest so, a == a1/a2/a3, and the test name can include forward slashes, so c == c1/c2/c3
// So we also need to support a1/a2/a3/b/c1/c2/c3
boolean limitTo3Separators = fullTestName.equals("logsdb/20_source_mapping/include/exclude is supported with stored _source");
String[] testParts = limitTo3Separators ? fullTestName.split("/", 3) : fullTestName.split("/");

String[] testParts = fullTestName.split("/");
if (testParts.length < 3) {
throw new IllegalArgumentException(
"To skip tests, all 3 parts [folder/file/test name] must be defined. found [" + fullTestName + "]"
Expand Down
10 changes: 0 additions & 10 deletions docs/changelog/116689.yaml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.common.network.InetAddresses;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.time.FormatNames;
import org.elasticsearch.test.MapMatcher;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
import org.elasticsearch.test.rest.RestTestLegacyFeatures;
Expand All @@ -30,6 +31,9 @@
import java.util.Map;
import java.util.function.Supplier;

import static org.elasticsearch.test.MapMatcher.assertMap;
import static org.elasticsearch.test.MapMatcher.matchesMap;

public class LogsIndexModeFullClusterRestartIT extends ParameterizedFullClusterRestartTestCase {

@ClassRule
Expand Down Expand Up @@ -168,16 +172,22 @@ public void testLogsIndexing() throws IOException {
assertOK(bulkIndexResponse);
assertThat(entityAsMap(bulkIndexResponse).get("errors"), Matchers.is(false));

assertIndexSettings(0, Matchers.nullValue());
assertIndexSettings(1, Matchers.equalTo("logsdb"));
assertIndexMappingsAndSettings(0, Matchers.nullValue(), matchesMap().extraOk());
assertIndexMappingsAndSettings(
1,
Matchers.equalTo("logsdb"),
matchesMap().extraOk().entry("_source", Map.of("mode", "synthetic"))
);
}
}

private void assertIndexSettings(int backingIndex, final Matcher<Object> indexModeMatcher) throws IOException {
private void assertIndexMappingsAndSettings(int backingIndex, final Matcher<Object> indexModeMatcher, final MapMatcher mappingsMatcher)
throws IOException {
assertThat(
getSettings(client(), getWriteBackingIndex(client(), "logs-apache-production", backingIndex)).get("index.mode"),
indexModeMatcher
);
assertMap(getIndexMappingAsMap(getWriteBackingIndex(client(), "logs-apache-production", backingIndex)), mappingsMatcher);
}

private static Request createDataStream(final String dataStreamName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.common.network.InetAddresses;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.time.FormatNames;
import org.elasticsearch.test.MapMatcher;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
import org.hamcrest.Matcher;
Expand All @@ -29,6 +30,9 @@
import java.util.Map;
import java.util.function.Supplier;

import static org.elasticsearch.test.MapMatcher.assertMap;
import static org.elasticsearch.test.MapMatcher.matchesMap;

public class LogsIndexModeRollingUpgradeIT extends AbstractRollingUpgradeTestCase {

@ClassRule()
Expand Down Expand Up @@ -156,10 +160,14 @@ public void testLogsIndexing() throws IOException {
assertOK(bulkIndexResponse);
assertThat(entityAsMap(bulkIndexResponse).get("errors"), Matchers.is(false));

assertIndexSettings(0, Matchers.nullValue());
assertIndexSettings(1, Matchers.nullValue());
assertIndexSettings(2, Matchers.nullValue());
assertIndexSettings(3, Matchers.equalTo("logsdb"));
assertIndexMappingsAndSettings(0, Matchers.nullValue(), matchesMap().extraOk());
assertIndexMappingsAndSettings(1, Matchers.nullValue(), matchesMap().extraOk());
assertIndexMappingsAndSettings(2, Matchers.nullValue(), matchesMap().extraOk());
assertIndexMappingsAndSettings(
3,
Matchers.equalTo("logsdb"),
matchesMap().extraOk().entry("_source", Map.of("mode", "synthetic"))
);
}
}

Expand All @@ -175,11 +183,13 @@ static void enableLogsdbByDefault() throws IOException {
assertOK(client().performRequest(request));
}

private void assertIndexSettings(int backingIndex, final Matcher<Object> indexModeMatcher) throws IOException {
private void assertIndexMappingsAndSettings(int backingIndex, final Matcher<Object> indexModeMatcher, final MapMatcher mappingsMatcher)
throws IOException {
assertThat(
getSettings(client(), getWriteBackingIndex(client(), "logs-apache-production", backingIndex)).get("index.mode"),
indexModeMatcher
);
assertMap(getIndexMappingAsMap(getWriteBackingIndex(client(), "logs-apache-production", backingIndex)), mappingsMatcher);
}

private static Request createDataStream(final String dataStreamName) {
Expand Down
6 changes: 0 additions & 6 deletions rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,4 @@ tasks.named("precommit").configure {
tasks.named("yamlRestTestV7CompatTransform").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/stored source is supported", "no longer serialize source_mode")
task.skipTest("tsdb/20_mapping/Synthetic source", "no longer serialize source_mode")
task.skipTest("logsdb/10_settings/create logs index", "no longer serialize source_mode")
task.skipTest("logsdb/20_source_mapping/stored _source mode is supported", "no longer serialize source_mode")
task.skipTest("logsdb/20_source_mapping/include/exclude is supported with stored _source", "no longer serialize source_mode")
task.skipTest("logsdb/20_source_mapping/synthetic _source is default", "no longer serialize source_mode")
})
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ create logs index:
- is_true: test
- match: { test.settings.index.mode: "logsdb" }

- do:
indices.get_mapping:
index: test
- match: { test.mappings._source.mode: synthetic }

---
using default timestamp field mapping:
- requires:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ synthetic _source is default:
index:
mode: logsdb
- do:
indices.get_settings:
indices.get:
index: test-default-source
- match: { test-default-source.settings.index.mode: logsdb }
- match: { test-default-source.settings.index.mapping.source.mode: null }

- match: { test-default-source.mappings._source.mode: "synthetic" }

---
stored _source mode is supported:
Expand All @@ -28,12 +28,11 @@ stored _source mode is supported:
index:
mode: logsdb
mapping.source.mode: stored

- do:
indices.get_settings:
indices.get:
index: test-stored-source
- match: { test-stored-source.settings.index.mode: logsdb }
- match: { test-stored-source.settings.index.mapping.source.mode: stored }

- match: { test-stored-source.mappings._source.mode: "stored" }

---
disabled _source is not supported:
Expand Down Expand Up @@ -111,6 +110,7 @@ include/exclude is supported with stored _source:
indices.get:
index: test-includes

- match: { test-includes.mappings._source.mode: "stored" }
- match: { test-includes.mappings._source.includes: ["a"] }

- do:
Expand All @@ -129,4 +129,5 @@ include/exclude is supported with stored _source:
indices.get:
index: test-excludes

- match: { test-excludes.mappings._source.mode: "stored" }
- match: { test-excludes.mappings._source.excludes: ["b"] }
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,11 @@ nested fields:
type: long
time_series_metric: gauge

- do:
indices.get_mapping: {}

- match: {tsdb-synthetic.mappings._source.mode: synthetic}

---
stored source is supported:
- requires:
Expand Down Expand Up @@ -481,6 +486,12 @@ stored source is supported:
type: keyword
time_series_dimension: true

- do:
indices.get:
index: tsdb_index

- match: { tsdb_index.mappings._source.mode: "stored" }

---
disabled source is not supported:
- requires:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.CheckedFunction;
Expand Down Expand Up @@ -55,7 +54,7 @@ Settings getAdditionalIndexSettings(
/**
* Infrastructure class that holds services that can be used by {@link IndexSettingProvider} instances.
*/
record Parameters(ClusterService clusterService, CheckedFunction<IndexMetadata, MapperService, IOException> mapperServiceFactory) {
record Parameters(CheckedFunction<IndexMetadata, MapperService, IOException> mapperServiceFactory) {

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ private static IndexVersion def(int id, Version luceneVersion) {
public static final IndexVersion ADD_ROLE_MAPPING_CLEANUP_MIGRATION = def(8_518_00_0, Version.LUCENE_9_12_0);
public static final IndexVersion LOGSDB_DEFAULT_IGNORE_DYNAMIC_BEYOND_LIMIT_BACKPORT = def(8_519_00_0, Version.LUCENE_9_12_0);
public static final IndexVersion TIME_BASED_K_ORDERED_DOC_ID_BACKPORT = def(8_520_00_0, Version.LUCENE_9_12_0);
public static final IndexVersion DEPRECATE_SOURCE_MODE_MAPPER = def(8_521_00_0, Version.LUCENE_9_12_0);
/*
* STOP! READ THIS FIRST! No, really,
* ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _
Expand Down
Loading

0 comments on commit 1bc60ac

Please sign in to comment.