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.

Sign up for GitHub

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

Jump to bottom

Make mappings immutable. #15313

Merged
merged 1 commit into from
Dec 15, 2015
Merged

Make mappings immutable. #15313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 2 additions & 6 deletions core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.elasticsearch.index.NodeServicesProvider;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MergeResult;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.indices.InvalidTypeNameException;
import org.elasticsearch.percolator.PercolatorService;
Expand Down Expand Up @@ -251,11 +250,8 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
newMapper = indexService.mapperService().parse(request.type(), new CompressedXContent(request.source()), existingMapper == null);
if (existingMapper != null) {
// first, simulate
MergeResult mergeResult = existingMapper.merge(newMapper.mapping(), true, request.updateAllTypes());
// if we have conflicts, throw an exception
if (mergeResult.hasConflicts()) {
throw new IllegalArgumentException("Merge failed with failures {" + Arrays.toString(mergeResult.buildConflicts()) + "}");
}
// this will just throw exceptions in case of problems
existingMapper.merge(newMapper.mapping(), true, request.updateAllTypes());
} else {
// TODO: can we find a better place for this validation?
// The reason this validation is here is that the mapper service doesn't learn about
Expand Down
16 changes: 10 additions & 6 deletions core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -117,7 +118,7 @@ public DocumentMapper build(MapperService mapperService, DocumentMapperParser do

private volatile CompressedXContent mappingSource;

private final Mapping mapping;
private volatile Mapping mapping;

private final DocumentParser documentParser;

Expand Down Expand Up @@ -352,16 +353,19 @@ private void addMappers(Collection<ObjectMapper> objectMappers, Collection<Field
mapperService.addMappers(type, objectMappers, fieldMappers);
}

public MergeResult merge(Mapping mapping, boolean simulate, boolean updateAllTypes) {
public void merge(Mapping mapping, boolean simulate, boolean updateAllTypes) {
try (ReleasableLock lock = mappingWriteLock.acquire()) {
mapperService.checkMappersCompatibility(type, mapping, updateAllTypes);
final MergeResult mergeResult = new MergeResult(simulate, updateAllTypes);
this.mapping.merge(mapping, mergeResult);
// do the merge even if simulate == false so that we get exceptions
Mapping merged = this.mapping.merge(mapping, updateAllTypes);
if (simulate == false) {
addMappers(mergeResult.getNewObjectMappers(), mergeResult.getNewFieldMappers(), updateAllTypes);
this.mapping = merged;
Collection<ObjectMapper> objectMappers = new ArrayList<>();
Collection<FieldMapper> fieldMappers = new ArrayList<>(Arrays.asList(merged.metadataMappers));
MapperUtils.collect(merged.root, objectMappers, fieldMappers);
addMappers(objectMappers, fieldMappers, updateAllTypes);
refreshSource();
}
return mergeResult;
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ static ObjectMapper parseObject(ParseContext context, ObjectMapper mapper, boole
if (update == null) {
update = newUpdate;
} else {
MapperUtils.merge(update, newUpdate);
update = update.merge(newUpdate, false);
}
}
}
Expand Down Expand Up @@ -759,7 +759,7 @@ private static void parseCopy(String field, ParseContext context) throws IOExcep
private static <M extends Mapper> M parseAndMergeUpdate(M mapper, ParseContext context) throws IOException {
final Mapper update = parseObjectOrField(context, mapper);
if (update != null) {
MapperUtils.merge(mapper, update);
mapper = (M) mapper.merge(update, false);
}
return mapper;
}
Expand Down
112 changes: 58 additions & 54 deletions core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
import java.util.Locale;
import java.util.stream.StreamSupport;

public abstract class FieldMapper extends Mapper {
public abstract class FieldMapper extends Mapper implements Cloneable {

public abstract static class Builder<T extends Builder, Y extends FieldMapper> extends Mapper.Builder<T, Y> {

Expand Down Expand Up @@ -84,8 +84,13 @@ public T index(boolean index) {
* if the fieldType has a non-null option we are all good it might have been set through a different
* call.
*/
final IndexOptions options = getDefaultIndexOption();
assert options != IndexOptions.NONE : "default IndexOptions is NONE can't enable indexing";
IndexOptions options = getDefaultIndexOption();
if (options == IndexOptions.NONE) {
// can happen when an existing type on the same index has disabled indexing
// since we inherit the default field type from the first mapper that is
// created on an index
throw new IllegalArgumentException("mapper [" + name + "] has different [index] values from other types of the same index");
}
fieldType.setIndexOptions(options);
}
} else {
Expand Down Expand Up @@ -270,7 +275,7 @@ protected void setupFieldType(BuilderContext context) {

protected MappedFieldTypeReference fieldTypeRef;
protected final MappedFieldType defaultFieldType;
protected final MultiFields multiFields;
protected MultiFields multiFields;
protected CopyTo copyTo;
protected final boolean indexCreatedBefore2x;

Expand Down Expand Up @@ -359,26 +364,41 @@ public Iterator<Mapper> iterator() {
}

@Override
public void merge(Mapper mergeWith, MergeResult mergeResult) {
protected FieldMapper clone() {
try {
return (FieldMapper) super.clone();
} catch (CloneNotSupportedException e) {
throw new AssertionError(e);
}
}

@Override
public FieldMapper merge(Mapper mergeWith, boolean updateAllTypes) {
FieldMapper merged = clone();
merged.doMerge(mergeWith, updateAllTypes);
return merged;
}

/**
* Merge changes coming from {@code mergeWith} in place.
* @param updateAllTypes TODO
*/
protected void doMerge(Mapper mergeWith, boolean updateAllTypes) {
if (!this.getClass().equals(mergeWith.getClass())) {
String mergedType = mergeWith.getClass().getSimpleName();
if (mergeWith instanceof FieldMapper) {
mergedType = ((FieldMapper) mergeWith).contentType();
}
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] of different type, current_type [" + contentType() + "], merged_type [" + mergedType + "]");
// different types, return
return;
throw new IllegalArgumentException("mapper [" + fieldType().names().fullName() + "] of different type, current_type [" + contentType() + "], merged_type [" + mergedType + "]");
}
FieldMapper fieldMergeWith = (FieldMapper) mergeWith;
multiFields.merge(mergeWith, mergeResult);
multiFields = multiFields.merge(fieldMergeWith.multiFields);

if (mergeResult.simulate() == false && mergeResult.hasConflicts() == false) {
// apply changeable values
MappedFieldType fieldType = fieldMergeWith.fieldType().clone();
fieldType.freeze();
fieldTypeRef.set(fieldType);
this.copyTo = fieldMergeWith.copyTo;
}
// apply changeable values
MappedFieldType fieldType = fieldMergeWith.fieldType().clone();
fieldType.freeze();
fieldTypeRef.set(fieldType);
this.copyTo = fieldMergeWith.copyTo;
}

@Override
Expand Down Expand Up @@ -565,18 +585,20 @@ public MultiFields build(FieldMapper.Builder mainFieldBuilder, BuilderContext co
}

private final ContentPath.Type pathType;
private volatile ImmutableOpenMap<String, FieldMapper> mappers;
private final ImmutableOpenMap<String, FieldMapper> mappers;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that!


public MultiFields(ContentPath.Type pathType, ImmutableOpenMap<String, FieldMapper> mappers) {
private MultiFields(ContentPath.Type pathType, ImmutableOpenMap<String, FieldMapper> mappers) {
this.pathType = pathType;
this.mappers = mappers;
ImmutableOpenMap.Builder<String, FieldMapper> builder = new ImmutableOpenMap.Builder<>();
// we disable the all in multi-field mappers
for (ObjectCursor<FieldMapper> cursor : mappers.values()) {
for (ObjectObjectCursor<String, FieldMapper> cursor : mappers) {
FieldMapper mapper = cursor.value;
if (mapper instanceof AllFieldMapper.IncludeInAll) {
((AllFieldMapper.IncludeInAll) mapper).unsetIncludeInAll();
mapper = (FieldMapper) ((AllFieldMapper.IncludeInAll) mapper).unsetIncludeInAll();
}
builder.put(cursor.key, mapper);
}
this.mappers = builder.build();
}

public void parse(FieldMapper mainField, ParseContext context) throws IOException {
Expand All @@ -598,47 +620,29 @@ public void parse(FieldMapper mainField, ParseContext context) throws IOExceptio
context.path().pathType(origPathType);
}

// No need for locking, because locking is taken care of in ObjectMapper#merge and DocumentMapper#merge
public void merge(Mapper mergeWith, MergeResult mergeResult) {
FieldMapper mergeWithMultiField = (FieldMapper) mergeWith;

List<FieldMapper> newFieldMappers = null;
ImmutableOpenMap.Builder<String, FieldMapper> newMappersBuilder = null;
public MultiFields merge(MultiFields mergeWith) {
if (pathType != mergeWith.pathType) {
throw new IllegalArgumentException("Can't change path type from [" + pathType + "] to [" + mergeWith.pathType + "]");
}
ImmutableOpenMap.Builder<String, FieldMapper> newMappersBuilder = ImmutableOpenMap.builder(mappers);

for (ObjectCursor<FieldMapper> cursor : mergeWithMultiField.multiFields.mappers.values()) {
for (ObjectCursor<FieldMapper> cursor : mergeWith.mappers.values()) {
FieldMapper mergeWithMapper = cursor.value;
Mapper mergeIntoMapper = mappers.get(mergeWithMapper.simpleName());
FieldMapper mergeIntoMapper = mappers.get(mergeWithMapper.simpleName());
if (mergeIntoMapper == null) {
// no mapping, simply add it if not simulating
if (!mergeResult.simulate()) {
// we disable the all in multi-field mappers
if (mergeWithMapper instanceof AllFieldMapper.IncludeInAll) {
((AllFieldMapper.IncludeInAll) mergeWithMapper).unsetIncludeInAll();
}
if (newMappersBuilder == null) {
newMappersBuilder = ImmutableOpenMap.builder(mappers);
}
newMappersBuilder.put(mergeWithMapper.simpleName(), mergeWithMapper);
if (mergeWithMapper instanceof FieldMapper) {
if (newFieldMappers == null) {
newFieldMappers = new ArrayList<>(2);
}
newFieldMappers.add(mergeWithMapper);
}
// we disable the all in multi-field mappers
if (mergeWithMapper instanceof AllFieldMapper.IncludeInAll) {
mergeWithMapper = (FieldMapper) ((AllFieldMapper.IncludeInAll) mergeWithMapper).unsetIncludeInAll();
}
newMappersBuilder.put(mergeWithMapper.simpleName(), mergeWithMapper);
} else {
mergeIntoMapper.merge(mergeWithMapper, mergeResult);
FieldMapper merged = mergeIntoMapper.merge(mergeWithMapper, false);
newMappersBuilder.put(merged.simpleName(), merged); // override previous definition
}
}

// first add all field mappers
if (newFieldMappers != null) {
mergeResult.addFieldMappers(newFieldMappers);
}
// now publish mappers
if (newMappersBuilder != null) {
mappers = newMappersBuilder.build();
}
ImmutableOpenMap<String, FieldMapper> mappers = newMappersBuilder.build();
return new MultiFields(pathType, mappers);
}

public Iterator<Mapper> iterator() {
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/org/elasticsearch/index/mapper/Mapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,5 +174,7 @@ public final String simpleName() {
/** Returns the canonical name which uniquely identifies the mapper against other mappers in a type. */
public abstract String name();

public abstract void merge(Mapper mergeWith, MergeResult mergeResult);
/** Return the merge of {@code mergeWith} into this.
* Both {@code this} and {@code mergeWith} will be left unmodified. */
public abstract Mapper merge(Mapper mergeWith, boolean updateAllTypes);
}
22 changes: 5 additions & 17 deletions core/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,7 @@ private DocumentMapper merge(DocumentMapper mapper, boolean updateAllTypes) {
DocumentMapper oldMapper = mappers.get(mapper.type());

if (oldMapper != null) {
// simulate first
MergeResult result = oldMapper.merge(mapper.mapping(), true, updateAllTypes);
if (result.hasConflicts()) {
throw new IllegalArgumentException("Merge failed with failures {" + Arrays.toString(result.buildConflicts()) + "}");
}
// then apply for real
result = oldMapper.merge(mapper.mapping(), false, updateAllTypes);
assert result.hasConflicts() == false; // we already simulated
oldMapper.merge(mapper.mapping(), false, updateAllTypes);
return oldMapper;
} else {
Tuple<Collection<ObjectMapper>, Collection<FieldMapper>> newMappers = checkMappersCompatibility(
Expand Down Expand Up @@ -305,12 +298,9 @@ protected void checkMappersCompatibility(String type, Collection<ObjectMapper> o
for (ObjectMapper newObjectMapper : objectMappers) {
ObjectMapper existingObjectMapper = fullPathObjectMappers.get(newObjectMapper.fullPath());
if (existingObjectMapper != null) {
MergeResult result = new MergeResult(true, updateAllTypes);
existingObjectMapper.merge(newObjectMapper, result);
if (result.hasConflicts()) {
throw new IllegalArgumentException("Mapper for [" + newObjectMapper.fullPath() + "] conflicts with existing mapping in other types" +
Arrays.toString(result.buildConflicts()));
}
// simulate a merge and ignore the result, we are just interested
// in exceptions here
existingObjectMapper.merge(newObjectMapper, updateAllTypes);
}
}
fieldTypes.checkCompatibility(type, fieldMappers, updateAllTypes);
Expand All @@ -320,9 +310,7 @@ protected Tuple<Collection<ObjectMapper>, Collection<FieldMapper>> checkMappersC
String type, Mapping mapping, boolean updateAllTypes) {
List<ObjectMapper> objectMappers = new ArrayList<>();
List<FieldMapper> fieldMappers = new ArrayList<>();
for (MetadataFieldMapper metadataMapper : mapping.metadataMappers) {
fieldMappers.add(metadataMapper);
}
Collections.addAll(fieldMappers, mapping.metadataMappers);
MapperUtils.collect(mapping.root, objectMappers, fieldMappers);
checkMappersCompatibility(type, objectMappers, fieldMappers, updateAllTypes);
return new Tuple<>(objectMappers, fieldMappers);
Expand Down
46 changes: 0 additions & 46 deletions core/src/main/java/org/elasticsearch/index/mapper/MapperUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,52 +27,6 @@
public enum MapperUtils {
;

private static MergeResult newStrictMergeResult() {
return new MergeResult(false, false) {

@Override
public void addFieldMappers(Collection<FieldMapper> fieldMappers) {
// no-op
}

@Override
public void addObjectMappers(Collection<ObjectMapper> objectMappers) {
// no-op
}

@Override
public Collection<FieldMapper> getNewFieldMappers() {
throw new UnsupportedOperationException("Strict merge result does not support new field mappers");
}

@Override
public Collection<ObjectMapper> getNewObjectMappers() {
throw new UnsupportedOperationException("Strict merge result does not support new object mappers");
}

@Override
public void addConflict(String mergeFailure) {
throw new MapperParsingException("Merging dynamic updates triggered a conflict: " + mergeFailure);
}
};
}

/**
* Merge {@code mergeWith} into {@code mergeTo}. Note: this method only
* merges mappings, not lookup structures. Conflicts are returned as exceptions.
*/
public static void merge(Mapper mergeInto, Mapper mergeWith) {
mergeInto.merge(mergeWith, newStrictMergeResult());
}

/**
* Merge {@code mergeWith} into {@code mergeTo}. Note: this method only
* merges mappings, not lookup structures. Conflicts are returned as exceptions.
*/
public static void merge(Mapping mergeInto, Mapping mergeWith) {
mergeInto.merge(mergeWith, newStrictMergeResult());
}

/** Split mapper and its descendants into object and field mappers. */
public static void collect(Mapper mapper, Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers) {
if (mapper instanceof RootObjectMapper) {
Expand Down
Loading