Skip to content

Commit

Permalink
[8.x] Returning ignored fields in the simulate ingest API (#117214) (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
masseyke authored Dec 24, 2024
1 parent 4d5503b commit eb0b8d6
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 14 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/117214.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 117214
summary: Returning ignored fields in the simulate ingest API
area: Ingest Node
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -1720,3 +1720,59 @@ setup:
- match: { docs.0.doc._source.foo: 3 }
- match: { docs.0.doc._source.bar: "some text value" }
- not_exists: docs.0.doc.error

---
"Test ignored_fields":
- skip:
features:
- headers
- allowed_warnings

- requires:
cluster_features: ["simulate.ignored.fields"]
reason: "ingest simulate ignored fields added in 8.18"

- do:
headers:
Content-Type: application/json
simulate.ingest:
index: nonexistent
body: >
{
"docs": [
{
"_index": "simulate-test",
"_id": "y9Es_JIBiw6_GgN-U0qy",
"_score": 1,
"_source": {
"abc": "sfdsfsfdsfsfdsfsfdsfsfdsfsfdsf"
}
}
],
"index_template_substitutions": {
"ind_temp": {
"index_patterns": ["simulate-test"],
"composed_of": ["simulate-test"]
}
},
"component_template_substitutions": {
"simulate-test": {
"template": {
"mappings": {
"dynamic": false,
"properties": {
"abc": {
"type": "keyword",
"ignore_above": 1
}
}
}
}
}
}
}
- length: { docs: 1 }
- match: { docs.0.doc._index: "simulate-test" }
- match: { docs.0.doc._source.abc: "sfdsfsfdsfsfdsfsfdsfsfdsfsfdsf" }
- match: { docs.0.doc.ignored_fields: [ {"field": "abc"} ] }
- not_exists: docs.0.doc.error
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ static TransportVersion def(int id) {
public static final TransportVersion NODE_VERSION_INFORMATION_WITH_MIN_READ_ONLY_INDEX_VERSION = def(8_810_00_0);
public static final TransportVersion ERROR_TRACE_IN_TRANSPORT_HEADER = def(8_811_00_0);
public static final TransportVersion FAILURE_STORE_ENABLED_BY_CLUSTER_SETTING = def(8_812_00_0);
public static final TransportVersion SIMULATE_IGNORED_FIELDS = def(8_813_00_0);

/*
* STOP! READ THIS FIRST! No, really,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.Set;

import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_COMPONENT_TEMPLATE_SUBSTITUTIONS;
import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_IGNORED_FIELDS;
import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_INDEX_TEMPLATE_SUBSTITUTIONS;
import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_MAPPING_ADDITION;
import static org.elasticsearch.action.bulk.TransportSimulateBulkAction.SIMULATE_MAPPING_VALIDATION;
Expand All @@ -29,7 +30,8 @@ public Set<NodeFeature> getFeatures() {
SIMULATE_COMPONENT_TEMPLATE_SUBSTITUTIONS,
SIMULATE_INDEX_TEMPLATE_SUBSTITUTIONS,
SIMULATE_MAPPING_ADDITION,
SIMULATE_SUPPORT_NON_TEMPLATE_MAPPING
SIMULATE_SUPPORT_NON_TEMPLATE_MAPPING,
SIMULATE_IGNORED_FIELDS
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

package org.elasticsearch.action.bulk;

import org.apache.lucene.document.StringField;
import org.apache.lucene.index.IndexableField;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.DocWriteRequest;
import org.elasticsearch.action.admin.indices.template.post.TransportSimulateIndexTemplateAction;
Expand All @@ -33,13 +35,16 @@
import org.elasticsearch.common.util.concurrent.AtomicArray;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.features.NodeFeature;
import org.elasticsearch.index.IndexSettingProvider;
import org.elasticsearch.index.IndexSettingProviders;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.IndexingPressure;
import org.elasticsearch.index.VersionType;
import org.elasticsearch.index.engine.Engine;
import org.elasticsearch.index.mapper.IgnoredFieldMapper;
import org.elasticsearch.index.mapper.LuceneDocument;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.SourceToParse;
import org.elasticsearch.index.seqno.SequenceNumbers;
Expand All @@ -60,6 +65,7 @@
import org.elasticsearch.xcontent.XContentType;

import java.io.IOException;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -85,6 +91,7 @@ public class TransportSimulateBulkAction extends TransportAbstractBulkAction {
public static final NodeFeature SIMULATE_INDEX_TEMPLATE_SUBSTITUTIONS = new NodeFeature("simulate.index.template.substitutions");
public static final NodeFeature SIMULATE_MAPPING_ADDITION = new NodeFeature("simulate.mapping.addition");
public static final NodeFeature SIMULATE_SUPPORT_NON_TEMPLATE_MAPPING = new NodeFeature("simulate.support.non.template.mapping");
public static final NodeFeature SIMULATE_IGNORED_FIELDS = new NodeFeature("simulate.ignored.fields");
private final IndicesService indicesService;
private final NamedXContentRegistry xContentRegistry;
private final Set<IndexSettingProvider> indexSettingProviders;
Expand Down Expand Up @@ -137,12 +144,13 @@ protected void doInternalExecute(
DocWriteRequest<?> docRequest = bulkRequest.requests.get(i);
assert docRequest instanceof IndexRequest : "TransportSimulateBulkAction should only ever be called with IndexRequests";
IndexRequest request = (IndexRequest) docRequest;
Exception mappingValidationException = validateMappings(
Tuple<Collection<String>, Exception> validationResult = validateMappings(
componentTemplateSubstitutions,
indexTemplateSubstitutions,
mappingAddition,
request
);
Exception mappingValidationException = validationResult.v2();
responses.set(
i,
BulkItemResponse.success(
Expand All @@ -155,6 +163,7 @@ protected void doInternalExecute(
request.source(),
request.getContentType(),
request.getExecutedPipelines(),
validationResult.v1(),
mappingValidationException
)
)
Expand All @@ -168,11 +177,12 @@ protected void doInternalExecute(
/**
* This creates a temporary index with the mappings of the index in the request, and then attempts to index the source from the request
* into it. If there is a mapping exception, that exception is returned. On success the returned exception is null.
* @parem componentTemplateSubstitutions The component template definitions to use in place of existing ones for validation
* @param componentTemplateSubstitutions The component template definitions to use in place of existing ones for validation
* @param request The IndexRequest whose source will be validated against the mapping (if it exists) of its index
* @return a mapping exception if the source does not match the mappings, otherwise null
* @return a Tuple containing: (1) in v1 the names of any fields that would be ignored upon indexing and (2) in v2 the mapping
* exception if the source does not match the mappings, otherwise null
*/
private Exception validateMappings(
private Tuple<Collection<String>, Exception> validateMappings(
Map<String, ComponentTemplate> componentTemplateSubstitutions,
Map<String, ComposableIndexTemplate> indexTemplateSubstitutions,
Map<String, Object> mappingAddition,
Expand All @@ -189,6 +199,7 @@ private Exception validateMappings(

ClusterState state = clusterService.state();
Exception mappingValidationException = null;
Collection<String> ignoredFields = List.of();
IndexAbstraction indexAbstraction = state.metadata().getIndicesLookup().get(request.index());
try {
if (indexAbstraction != null
Expand Down Expand Up @@ -275,7 +286,7 @@ private Exception validateMappings(
);
CompressedXContent mappings = template.mappings();
CompressedXContent mergedMappings = mergeMappings(mappings, mappingAddition);
validateUpdatedMappings(mappings, mergedMappings, request, sourceToParse);
ignoredFields = validateUpdatedMappings(mappings, mergedMappings, request, sourceToParse);
} else {
List<IndexTemplateMetadata> matchingTemplates = findV1Templates(simulatedState.metadata(), request.index(), false);
if (matchingTemplates.isEmpty() == false) {
Expand All @@ -289,7 +300,7 @@ private Exception validateMappings(
xContentRegistry
);
final CompressedXContent combinedMappings = mergeMappings(new CompressedXContent(mappingsMap), mappingAddition);
validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
ignoredFields = validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
} else if (indexAbstraction != null && mappingAddition.isEmpty() == false) {
/*
* The index matched no templates of any kind, including the substitutions. But it might have a mapping. So we
Expand All @@ -298,35 +309,36 @@ private Exception validateMappings(
MappingMetadata mappingFromIndex = clusterService.state().metadata().index(indexAbstraction.getName()).mapping();
CompressedXContent currentIndexCompressedXContent = mappingFromIndex == null ? null : mappingFromIndex.source();
CompressedXContent combinedMappings = mergeMappings(currentIndexCompressedXContent, mappingAddition);
validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
ignoredFields = validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
} else {
/*
* The index matched no templates and had no mapping of its own. If there were component template substitutions
* or index template substitutions, they didn't match anything. So just apply the mapping addition if it exists,
* and validate.
*/
final CompressedXContent combinedMappings = mergeMappings(null, mappingAddition);
validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
ignoredFields = validateUpdatedMappings(null, combinedMappings, request, sourceToParse);
}
}
}
} catch (Exception e) {
mappingValidationException = e;
}
return mappingValidationException;
return Tuple.tuple(ignoredFields, mappingValidationException);
}

/*
* Validates that when updatedMappings are applied
* Validates that when updatedMappings are applied. If any fields would be ignored while indexing, then those field names are returned.
* Otherwise the returned Collection is empty.
*/
private void validateUpdatedMappings(
private Collection<String> validateUpdatedMappings(
@Nullable CompressedXContent originalMappings,
@Nullable CompressedXContent updatedMappings,
IndexRequest request,
SourceToParse sourceToParse
) throws IOException {
if (updatedMappings == null) {
return; // no validation to do
return List.of(); // no validation to do
}
Settings dummySettings = Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
Expand All @@ -343,7 +355,7 @@ private void validateUpdatedMappings(
.settings(dummySettings)
.putMapping(new MappingMetadata(updatedMappings))
.build();
indicesService.withTempIndexService(originalIndexMetadata, indexService -> {
Engine.Index result = indicesService.withTempIndexService(originalIndexMetadata, indexService -> {
indexService.mapperService().merge(updatedIndexMetadata, MapperService.MergeReason.MAPPING_UPDATE);
return IndexShard.prepareIndex(
indexService.mapperService(),
Expand All @@ -360,6 +372,24 @@ private void validateUpdatedMappings(
0
);
});
final Collection<String> ignoredFields;
if (result == null) {
ignoredFields = List.of();
} else {
List<LuceneDocument> luceneDocuments = result.parsedDoc().docs();
assert luceneDocuments == null || luceneDocuments.size() == 1 : "Expected a single lucene document from index attempt";
if (luceneDocuments != null && luceneDocuments.size() == 1) {
ignoredFields = luceneDocuments.get(0)
.getFields()
.stream()
.filter(field -> field.name().equals(IgnoredFieldMapper.NAME) && field instanceof StringField)
.map(IndexableField::stringValue)
.toList();
} else {
ignoredFields = List.of();
}
}
return ignoredFields;
}

private static CompressedXContent mergeMappings(@Nullable CompressedXContent originalMapping, Map<String, Object> mappingAddition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.xcontent.XContentType;

import java.io.IOException;
import java.util.Collection;
import java.util.List;

/**
Expand All @@ -34,6 +35,7 @@
public class SimulateIndexResponse extends IndexResponse {
private final BytesReference source;
private final XContentType sourceXContentType;
private final Collection<String> ignoredFields;
private final Exception exception;

@SuppressWarnings("this-escape")
Expand All @@ -47,6 +49,11 @@ public SimulateIndexResponse(StreamInput in) throws IOException {
} else {
this.exception = null;
}
if (in.getTransportVersion().onOrAfter(TransportVersions.SIMULATE_IGNORED_FIELDS)) {
this.ignoredFields = in.readStringCollectionAsList();
} else {
this.ignoredFields = List.of();
}
}

@SuppressWarnings("this-escape")
Expand All @@ -57,6 +64,7 @@ public SimulateIndexResponse(
BytesReference source,
XContentType sourceXContentType,
List<String> pipelines,
Collection<String> ignoredFields,
@Nullable Exception exception
) {
// We don't actually care about most of the IndexResponse fields:
Expand All @@ -73,6 +81,7 @@ public SimulateIndexResponse(
this.source = source;
this.sourceXContentType = sourceXContentType;
setShardInfo(ShardInfo.EMPTY);
this.ignoredFields = ignoredFields;
this.exception = exception;
}

Expand All @@ -84,6 +93,16 @@ public XContentBuilder innerToXContent(XContentBuilder builder, Params params) t
builder.field("_source", XContentHelper.convertToMap(source, false, sourceXContentType).v2());
assert executedPipelines != null : "executedPipelines is null when it shouldn't be - we always list pipelines in simulate mode";
builder.array("executed_pipelines", executedPipelines.toArray());
if (ignoredFields.isEmpty() == false) {
builder.startArray("ignored_fields");
for (String ignoredField : ignoredFields) {
builder.startObject();
builder.field("field", ignoredField);
builder.endObject();
}
;
builder.endArray();
}
if (exception != null) {
builder.startObject("error");
ElasticsearchException.generateThrowableXContent(builder, params, exception);
Expand All @@ -105,6 +124,9 @@ public void writeTo(StreamOutput out) throws IOException {
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_15_0)) {
out.writeException(exception);
}
if (out.getTransportVersion().onOrAfter(TransportVersions.SIMULATE_IGNORED_FIELDS)) {
out.writeStringCollection(ignoredFields);
}
}

public Exception getException() {
Expand Down
Loading

0 comments on commit eb0b8d6

Please sign in to comment.