Skip to content
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
10 changes: 10 additions & 0 deletions docs/changelog/144040.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
area: Mapping
issues:
- 142544
- 142964
- 142410
- 143884
- 142477
pr: 144040
summary: Drop deprecation warnings when updating a mapping in the cluster state applier
type: bug
3 changes: 0 additions & 3 deletions muted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,6 @@ tests:
- class: org.elasticsearch.xpack.esql.qa.single_node.EsqlSpecIT
method: test {csv-spec:approximation.Approximate stats with sample}
issue: https://github.com/elastic/elasticsearch/issues/144022
- class: org.elasticsearch.xpack.esql.qa.multi_node.EsqlClientYamlIT
method: test {p0=esql/40_tsdb/to_aggregate_metric_double with multi_values}
issue: https://github.com/elastic/elasticsearch/issues/142964
- class: org.elasticsearch.repositories.azure.AzureBlobContainerRetriesTests
method: testWriteBlobWithRetries
issue: https://github.com/elastic/elasticsearch/issues/144025
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,12 @@ List<SearchOperationListener> getSearchOperationListener() { // pkg private for

public void updateMapping(final IndexMetadata currentIndexMetadata, final IndexMetadata newIndexMetadata) {
if (mapperService != null) {
mapperService.updateMapping(currentIndexMetadata, newIndexMetadata);
// Mapper service re-parses the mapping here, potentially adding deprecation warning response headers, but we're in the process
// of applying the cluster state and have no way to send these warnings back to the client so we just drop them. We will have
// generated, and sent back, the same headers while handling the request that created this mapping in the first place.
try (var ignored = threadPool.getThreadContext().newStoredContext()) {
mapperService.updateMapping(currentIndexMetadata, newIndexMetadata);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.index.LeafReaderContext;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.TriFunction;
Expand Down Expand Up @@ -1940,13 +1939,6 @@ public final void parse(String name, MappingParserContext parserContext, Map<Str
);
}
if (parameter.deprecated) {
// Remove the stack track trace logging after https://github.com/elastic/elasticsearch/issues/143884
if (logger.isDebugEnabled() && "default_metric".equals(propName)) {
logger.debug(
"Parsing [" + contentType() + "] with deprecated [default_metric] config",
new ElasticsearchException("Stack trace retrieval")
);
}
deprecationLogger.warn(
DeprecationCategory.API,
propName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
package org.elasticsearch.indices;

import org.apache.lucene.search.Query;
import org.apache.lucene.search.similarities.BM25Similarity;
import org.apache.lucene.search.similarities.Similarity;
import org.apache.lucene.store.AlreadyClosedException;
Expand Down Expand Up @@ -56,11 +57,18 @@
import org.elasticsearch.index.engine.InternalEngine;
import org.elasticsearch.index.engine.InternalEngineFactory;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.DocumentParserContext;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.IndexType;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperBuilderContext;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.ValueFetcher;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.index.shard.IllegalIndexShardStateException;
import org.elasticsearch.index.shard.IndexShard;
import org.elasticsearch.index.shard.IndexShardState;
Expand Down Expand Up @@ -96,6 +104,7 @@
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.empty;
Expand Down Expand Up @@ -194,13 +203,98 @@ public Optional<EngineFactory> getEngineFactory(final IndexSettings indexSetting

}

static class DeprecatedParameterMapper extends FieldMapper {

static final String CONTENT_TYPE = "deprecated-param-mapper";

private DeprecatedParameterMapper(String simpleName, MappedFieldType mappedFieldType, BuilderParams builderParams) {
super(simpleName, mappedFieldType, builderParams);
}

@Override
protected void parseCreateField(DocumentParserContext context) {}

@Override
public Builder getMergeBuilder() {
return new DeprecatedParameterMapper.TypeBuilder(leafName()).init(this);
}

@Override
protected String contentType() {
return CONTENT_TYPE;
}

static class TypeBuilder extends FieldMapper.Builder {

private final Parameter<String> deprecatedParam = Parameter.stringParam(
"deprecated_field",
true,
m -> ((DeprecatedParameterFieldType) m.fieldType()).deprecatedField,
null
).acceptsNull().deprecated();

TypeBuilder(String name) {
super(name);
}

@Override
protected Parameter<?>[] getParameters() {
return new Parameter<?>[] { deprecatedParam };
}

@Override
public String contentType() {
return CONTENT_TYPE;
}

@Override
public FieldMapper build(MapperBuilderContext context) {
return new DeprecatedParameterMapper(
leafName(),
new DeprecatedParameterFieldType(context.buildFullName(leafName()), deprecatedParam.getValue()),
builderParams(this, context)
);
}
}

static class DeprecatedParameterFieldType extends MappedFieldType {

private final String deprecatedField;

DeprecatedParameterFieldType(String name, String deprecatedField) {
super(name, IndexType.NONE, false, Map.of());
this.deprecatedField = deprecatedField;
}

@Override
public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
return ValueFetcher.EMPTY;
}

@Override
public String typeName() {
return CONTENT_TYPE;
}

@Override
public Query termQuery(Object value, SearchExecutionContext context) {
throw new UnsupportedOperationException();
}
}
}

public static class TestPlugin extends Plugin implements MapperPlugin {

public TestPlugin() {}

@Override
public Map<String, Mapper.TypeParser> getMappers() {
return Collections.singletonMap("fake-mapper", KeywordFieldMapper.PARSER);
return Map.of(
"fake-mapper",
KeywordFieldMapper.PARSER,
DeprecatedParameterMapper.CONTENT_TYPE,
new FieldMapper.TypeParser((name, parserContext) -> new DeprecatedParameterMapper.TypeBuilder(name))
);
}

@Override
Expand Down Expand Up @@ -940,6 +1034,45 @@ public void testMapperServiceForValidationChecksMapping() throws IOException {
assertNull(temporaryDocumentMapper);
}

/**
* Tests that deprecations warnings do not leak when the mapping is updated
*/
public void testDeprecationsWarningsDoNotEscape() throws IOException {
IndicesService indicesService = getIndicesService();
IndexMetadata initialIndexMetadata = IndexMetadata.builder("test")
.settings(indexSettings(IndexVersion.current(), randomUUID(), 1, 0))
.build();
IndexService indexService = indicesService.createIndex(initialIndexMetadata, List.of(), randomBoolean());

IndexMetadata newIndexMetadata = IndexMetadata.builder(initialIndexMetadata)
.mappingVersion(initialIndexMetadata.getMappingVersion() + 1)
.putMapping("""
{
"_doc":{
"properties": {
"my-field": {
"type": "deprecated-param-mapper",
"deprecated_field": "someValue"
}
}
}
}""")
.build();
indexService.updateMapping(initialIndexMetadata, newIndexMetadata);
// This test sets two thread contexts to the HeaderWarning class, the thread context of the ESTestCase and the
// thread pool one, consequently the deprecation warning is added to both. The production code will only have
// the thread pool context, so we test the isolation of this context only.
final List<String> warnings = indexService.getThreadPool().getThreadContext().getResponseHeaders().get("Warning");
if (warnings != null) {
assertThat(
warnings,
not(contains(containsString("Parameter [deprecated_field] is deprecated and will be removed in a future version")))
);
}
// For the test's thread context we just handle the expected warning.
assertWarnings("Parameter [deprecated_field] is deprecated and will be removed in a future version");
}

private Set<ResolvedExpression> resolvedExpressions(String... expressions) {
return Arrays.stream(expressions).map(ResolvedExpression::new).collect(Collectors.toSet());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ public static ElasticsearchCluster mixedVersionCluster() {
.withNode(node -> node.version(oldVersionString, isDetachedVersion))
.withNode(node -> node.version(Version.CURRENT))
.setting("xpack.security.enabled", "false")
.setting("xpack.license.self_generated.type", "trial")
// Remove after https://github.com/elastic/elasticsearch/issues/143884 (only the current version will log the stack trace)
.setting("logger.org.elasticsearch.index.mapper.FieldMapper", "DEBUG");
.setting("xpack.license.self_generated.type", "trial");
if (supportRetryOnShardFailures(oldVersion) == false) {
cluster.setting("cluster.routing.rebalance.enable", "none");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@
@ThreadLeakFilters(filters = TestClustersThreadFilter.class)
public class FieldExtractorIT extends FieldExtractorTestCase {
@ClassRule
public static ElasticsearchCluster cluster = Clusters.testCluster(
// Remove after https://github.com/elastic/elasticsearch/issues/143884
spec -> spec.setting("logger.org.elasticsearch.index.mapper.FieldMapper", "DEBUG")
);
public static ElasticsearchCluster cluster = Clusters.testCluster(spec -> {});

public FieldExtractorIT(MappedFieldType.FieldExtractPreference preference) {
super(preference);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.WarningFailureException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.network.NetworkAddress;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -1412,30 +1411,19 @@ public void testTsIndexConflictingTypes() throws IOException {
}
}
""", null, DEPRECATED_DEFAULT_METRIC_WARNING_HANDLER);
// Remove try-catch after https://github.com/elastic/elasticsearch/issues/143884
try {
ESRestTestCase.createIndex("metrics-long", settings, """
"properties": {
"@timestamp": { "type": "date" },
"metric.name": {
"type": "keyword",
"time_series_dimension": true
},
"metric.value": {
"type": "long",
"time_series_metric": "gauge"
}
}
""");
} catch (WarningFailureException warningException) {
List<String> warnings = warningException.getResponse().getWarnings();
if (warnings.stream()
.anyMatch(warning -> warning.equals("Parameter [default_metric] is deprecated and will be removed in a future version"))) {
Map<String, Object> indexMapping = ESRestTestCase.getIndexMapping("metrics-long");
logger.error("Received warning when creating index [metrics-long] with mapping [{}]", indexMapping);
ESRestTestCase.createIndex("metrics-long", settings, """
"properties": {
"@timestamp": { "type": "date" },
"metric.name": {
"type": "keyword",
"time_series_dimension": true
},
"metric.value": {
"type": "long",
"time_series_metric": "gauge"
}
}
throw warningException;
}
""");
ESRestTestCase.createIndex("metrics-long_dimension", settings, """
"properties": {
"@timestamp": { "type": "date" },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
setup:
- requires:
test_runner_features: [allowed_warnings_regex, allowed_warnings]
# Remove enabling debug logging after https://github.com/elastic/elasticsearch/issues/143884
- do:
cluster.put_settings:
body: >
{
"persistent": {
"logger.org.elasticsearch.index.mapper.FieldMapper": "DEBUG"
}
}
- do:
indices.create:
index: test
Expand Down
Loading