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
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
public class PutMappingClusterStateUpdateRequest extends IndicesClusterStateUpdateRequest<PutMappingClusterStateUpdateRequest> {

private final CompressedXContent source;
private boolean autoUpdate;

public PutMappingClusterStateUpdateRequest(String source) throws IOException {
this.source = CompressedXContent.fromJSON(source);
Expand All @@ -28,4 +29,12 @@ public CompressedXContent source() {
return source;
}

public PutMappingClusterStateUpdateRequest autoUpdate(boolean autoUpdate) {
this.autoUpdate = autoUpdate;
return this;
}

public boolean autoUpdate() {
return autoUpdate;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ protected void masterOperation(
return;
}

performMappingUpdate(concreteIndices, request, listener, metadataMappingService);
performMappingUpdate(concreteIndices, request, listener, metadataMappingService, true);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ protected void masterOperation(
return;
}

performMappingUpdate(concreteIndices, request, listener, metadataMappingService);
performMappingUpdate(concreteIndices, request, listener, metadataMappingService, false);
} catch (IndexNotFoundException ex) {
logger.debug(() -> "failed to put mappings on indices [" + Arrays.asList(request.indices() + "]"), ex);
throw ex;
Expand Down Expand Up @@ -147,7 +147,8 @@ static void performMappingUpdate(
Index[] concreteIndices,
PutMappingRequest request,
ActionListener<AcknowledgedResponse> listener,
MetadataMappingService metadataMappingService
MetadataMappingService metadataMappingService,
boolean autoUpdate
) {
final ActionListener<AcknowledgedResponse> wrappedListener = listener.delegateResponse((l, e) -> {
logger.debug(() -> "failed to put mappings on indices [" + Arrays.asList(concreteIndices) + "]", e);
Expand All @@ -157,7 +158,8 @@ static void performMappingUpdate(
try {
updateRequest = new PutMappingClusterStateUpdateRequest(request.source()).indices(concreteIndices)
.ackTimeout(request.timeout())
.masterNodeTimeout(request.masterNodeTimeout());
.masterNodeTimeout(request.masterNodeTimeout())
.autoUpdate(autoUpdate);
} catch (IOException e) {
wrappedListener.onFailure(e);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ static boolean executeBulkItemRequest(
.merge(
MapperService.SINGLE_MAPPING_NAME,
new CompressedXContent(result.getRequiredMappingUpdate()),
MapperService.MergeReason.MAPPING_UPDATE_PREFLIGHT
MapperService.MergeReason.MAPPING_AUTO_UPDATE_PREFLIGHT
)
).map(DocumentMapper::mappingSource);
Optional<CompressedXContent> previousSource = Optional.ofNullable(primary.mapperService().documentMapper())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ private static ClusterState applyRequest(
// try and parse it (no need to add it here) so we can bail early in case of parsing exception
// first, simulate: just call merge and ignore the result
Mapping mapping = mapperService.parseMapping(MapperService.SINGLE_MAPPING_NAME, mappingUpdateSource);
MapperService.mergeMappings(mapperService.documentMapper(), mapping, MergeReason.MAPPING_UPDATE);
MapperService.mergeMappings(
mapperService.documentMapper(),
mapping,
request.autoUpdate() ? MergeReason.MAPPING_AUTO_UPDATE : MergeReason.MAPPING_UPDATE
);
}
Metadata.Builder builder = Metadata.builder(metadata);
boolean updated = false;
Expand All @@ -167,7 +171,7 @@ private static ClusterState applyRequest(
DocumentMapper mergedMapper = mapperService.merge(
MapperService.SINGLE_MAPPING_NAME,
mappingUpdateSource,
MergeReason.MAPPING_UPDATE
request.autoUpdate() ? MergeReason.MAPPING_AUTO_UPDATE : MergeReason.MAPPING_UPDATE
Comment thread
henningandersen marked this conversation as resolved.
);
CompressedXContent updatedSource = mergedMapper.mappingSource();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,23 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
*/
public enum MergeReason {
/**
* Pre-flight check before sending a mapping update to the master
* Pre-flight check before sending a dynamic mapping update to the master
*/
MAPPING_UPDATE_PREFLIGHT,
MAPPING_AUTO_UPDATE_PREFLIGHT {
@Override
public boolean isAutoUpdate() {
return true;
}
},
/**
* Dynamic mapping updates
*/
MAPPING_AUTO_UPDATE {
Comment thread
henningandersen marked this conversation as resolved.
@Override
public boolean isAutoUpdate() {
return true;
}
},
/**
* Create or update a mapping.
*/
Expand All @@ -72,7 +86,11 @@ public enum MergeReason {
* if a shard was moved to a different node or for administrative
* purposes.
*/
MAPPING_RECOVERY
MAPPING_RECOVERY;

public boolean isAutoUpdate() {
return false;
}
}

public static final String SINGLE_MAPPING_NAME = "_doc";
Expand Down Expand Up @@ -364,7 +382,7 @@ boolean assertNoUpdateRequired(final IndexMetadata newIndexMetadata) {
}

public void merge(IndexMetadata indexMetadata, MergeReason reason) {
assert reason != MergeReason.MAPPING_UPDATE_PREFLIGHT;
assert reason != MergeReason.MAPPING_AUTO_UPDATE_PREFLIGHT;
MappingMetadata mappingMetadata = indexMetadata.mapping();
if (mappingMetadata != null) {
merge(mappingMetadata.type(), mappingMetadata.source(), reason);
Expand Down Expand Up @@ -521,7 +539,7 @@ private synchronized DocumentMapper doMerge(String type, MergeReason reason, Map
// TODO: In many cases the source here is equal to mappingSource so we need not serialize again.
// We should identify these cases reliably and save expensive serialization here
DocumentMapper newMapper = newDocumentMapper(mapping, reason, mapping.toCompressedXContent());
if (reason == MergeReason.MAPPING_UPDATE_PREFLIGHT) {
if (reason == MergeReason.MAPPING_AUTO_UPDATE_PREFLIGHT) {
return newMapper;
}
this.mapper = newMapper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void addDynamicMappingsUpdate(Mapping update) {
if (dynamicMappingsUpdate == null) {
dynamicMappingsUpdate = update;
} else {
dynamicMappingsUpdate = dynamicMappingsUpdate.merge(update, MergeReason.MAPPING_UPDATE, Long.MAX_VALUE);
dynamicMappingsUpdate = dynamicMappingsUpdate.merge(update, MergeReason.MAPPING_AUTO_UPDATE, Long.MAX_VALUE);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void testAddFields() throws Exception {
b.endObject();
}));

MergeReason reason = randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.INDEX_TEMPLATE);
MergeReason reason = randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.INDEX_TEMPLATE, MergeReason.MAPPING_AUTO_UPDATE);
Mapping merged = MapperService.mergeMappings(stage1, stage2.mapping(), reason, Long.MAX_VALUE);
// stage1 mapping should not have been modified
assertThat(stage1.mappers().getMapper("age"), nullValue());
Expand All @@ -81,14 +81,19 @@ public void testMergeObjectDynamic() throws Exception {
DocumentMapper withDynamicMapper = createDocumentMapper(topMapping(b -> b.field("dynamic", "false")));
assertThat(withDynamicMapper.mapping().getRoot().dynamic(), equalTo(ObjectMapper.Dynamic.FALSE));

Mapping merged = MapperService.mergeMappings(mapper, withDynamicMapper.mapping(), MergeReason.MAPPING_UPDATE, Long.MAX_VALUE);
Mapping merged = MapperService.mergeMappings(
mapper,
withDynamicMapper.mapping(),
randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.MAPPING_AUTO_UPDATE),
Long.MAX_VALUE
);
assertThat(merged.getRoot().dynamic(), equalTo(ObjectMapper.Dynamic.FALSE));
}

public void testMergeObjectAndNested() throws Exception {
DocumentMapper objectMapper = createDocumentMapper(mapping(b -> b.startObject("obj").field("type", "object").endObject()));
DocumentMapper nestedMapper = createDocumentMapper(mapping(b -> b.startObject("obj").field("type", "nested").endObject()));
MergeReason reason = randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.INDEX_TEMPLATE);
MergeReason reason = randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.INDEX_TEMPLATE, MergeReason.MAPPING_AUTO_UPDATE);

{
IllegalArgumentException e = expectThrows(
Expand Down Expand Up @@ -178,7 +183,11 @@ public void testConcurrentMergeTest() throws Throwable {
Mapping update = doc.dynamicMappingsUpdate();
assert update != null;
lastIntroducedFieldName.set(fieldName);
mapperService.merge("_doc", new CompressedXContent(update.toString()), MergeReason.MAPPING_UPDATE);
mapperService.merge(
"_doc",
new CompressedXContent(update.toString()),
randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.MAPPING_AUTO_UPDATE)
);
}
} catch (Exception e) {
error.set(e);
Expand Down Expand Up @@ -235,11 +244,21 @@ public void testMergeMeta() throws IOException {

DocumentMapper updatedMapper = createDocumentMapper(fieldMapping(b -> b.field("type", "text")));

Mapping merged = MapperService.mergeMappings(initMapper, updatedMapper.mapping(), MergeReason.MAPPING_UPDATE, Long.MAX_VALUE);
Mapping merged = MapperService.mergeMappings(
initMapper,
updatedMapper.mapping(),
randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.MAPPING_AUTO_UPDATE),
Long.MAX_VALUE
);
assertThat(merged.getMeta().get("foo"), equalTo("bar"));

updatedMapper = createDocumentMapper(topMapping(b -> b.startObject("_meta").field("foo", "new_bar").endObject()));
merged = MapperService.mergeMappings(initMapper, updatedMapper.mapping(), MergeReason.MAPPING_UPDATE, Long.MAX_VALUE);
merged = MapperService.mergeMappings(
initMapper,
updatedMapper.mapping(),
randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.MAPPING_AUTO_UPDATE),
Long.MAX_VALUE
);
assertThat(merged.getMeta().get("foo"), equalTo("new_bar"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,13 @@ public class MapperServiceTests extends MapperServiceTestCase {

public void testPreflightUpdateDoesNotChangeMapping() throws Throwable {
final MapperService mapperService = createMapperService(mapping(b -> {}));
merge(mapperService, MergeReason.MAPPING_UPDATE_PREFLIGHT, mapping(b -> createMappingSpecifyingNumberOfFields(b, 1)));
merge(mapperService, MergeReason.MAPPING_AUTO_UPDATE_PREFLIGHT, mapping(b -> createMappingSpecifyingNumberOfFields(b, 1)));
assertThat("field was not created by preflight check", mapperService.fieldType("field0"), nullValue());
merge(mapperService, MergeReason.MAPPING_UPDATE, mapping(b -> createMappingSpecifyingNumberOfFields(b, 1)));
merge(
mapperService,
randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.MAPPING_AUTO_UPDATE),
mapping(b -> createMappingSpecifyingNumberOfFields(b, 1))
);
assertThat("field was not created by mapping update", mapperService.fieldType("field0"), notNullValue());
}

Expand Down