Skip to content

Commit

Permalink
Merge pull request #12371 from jpountz/fix/mapper_fielddata_dependency
Browse files Browse the repository at this point in the history 
Remove the dependecy on IndexFielddataService from MapperService.
  • Loading branch information
@jpountz
jpountz committed Jul 22, 2015
2 parents 828d8c7 + 3fd7a36 commit f8b741d
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 85 deletions.
117 changes: 39 additions & 78 deletions core/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
import org.elasticsearch.common.util.concurrent.KeyedLock;
import org.elasticsearch.index.AbstractIndexComponent;
import org.elasticsearch.index.Index;
Expand All @@ -44,7 +43,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentMap;

import static org.elasticsearch.index.mapper.MappedFieldType.Names;

Expand Down Expand Up @@ -138,7 +136,6 @@ public class IndexFieldDataService extends AbstractIndexComponent {
}

private final IndicesFieldDataCache indicesFieldDataCache;
private final ConcurrentMap<String, IndexFieldData<?>> loadedFieldData = ConcurrentCollections.newConcurrentMap();
private final KeyedLock.GlobalLockable<String> fieldLoadingLock = new KeyedLock.GlobalLockable<>();
private final Map<String, IndexFieldDataCache> fieldDataCaches = Maps.newHashMap(); // no need for concurrency support, always used under lock

Expand All @@ -161,15 +158,6 @@ public void clear() {
fieldLoadingLock.globalLock().lock();
try {
List<Throwable> exceptions = new ArrayList<>(0);
final Collection<IndexFieldData<?>> fieldDataValues = loadedFieldData.values();
for (IndexFieldData<?> fieldData : fieldDataValues) {
try {
fieldData.clear();
} catch (Throwable t) {
exceptions.add(t);
}
}
fieldDataValues.clear();
final Collection<IndexFieldDataCache> fieldDataCacheValues = fieldDataCaches.values();
for (IndexFieldDataCache cache : fieldDataCacheValues) {
try {
Expand All @@ -189,14 +177,6 @@ public void clearField(final String fieldName) {
fieldLoadingLock.acquire(fieldName);
try {
List<Throwable> exceptions = new ArrayList<>(0);
final IndexFieldData<?> fieldData = loadedFieldData.remove(fieldName);
if (fieldData != null) {
try {
fieldData.clear();
} catch (Throwable t) {
exceptions.add(t);
}
}
final IndexFieldDataCache cache = fieldDataCaches.remove(fieldName);
if (cache != null) {
try {
Expand All @@ -211,17 +191,6 @@ public void clearField(final String fieldName) {
}
}

public void onMappingUpdate() {
// synchronize to make sure to not miss field data instances that are being loaded
fieldLoadingLock.globalLock().lock();
try {
// important: do not clear fieldDataCaches: the cache may be reused
loadedFieldData.clear();
} finally {
fieldLoadingLock.globalLock().unlock();
}
}

@SuppressWarnings("unchecked")
public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
final Names fieldNames = fieldType.names();
Expand All @@ -231,57 +200,49 @@ public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType
}
final boolean docValues = fieldType.hasDocValues();
final String key = fieldNames.indexName();
IndexFieldData<?> fieldData = loadedFieldData.get(key);
if (fieldData == null) {
fieldLoadingLock.acquire(key);
try {
fieldData = loadedFieldData.get(key);
if (fieldData == null) {
IndexFieldData.Builder builder = null;
String format = type.getFormat(indexSettings);
if (format != null && FieldDataType.DOC_VALUES_FORMAT_VALUE.equals(format) && !docValues) {
logger.warn("field [" + fieldNames.fullName() + "] has no doc values, will use default field data format");
format = null;
}
if (format != null) {
builder = buildersByTypeAndFormat.get(Tuple.tuple(type.getType(), format));
if (builder == null) {
logger.warn("failed to find format [" + format + "] for field [" + fieldNames.fullName() + "], will use default");
}
}
if (builder == null && docValues) {
builder = docValuesBuildersByType.get(type.getType());
}
if (builder == null) {
builder = buildersByType.get(type.getType());
}
if (builder == null) {
throw new IllegalArgumentException("failed to find field data builder for field " + fieldNames.fullName() + ", and type " + type.getType());
}

IndexFieldDataCache cache = fieldDataCaches.get(fieldNames.indexName());
if (cache == null) {
// we default to node level cache, which in turn defaults to be unbounded
// this means changing the node level settings is simple, just set the bounds there
String cacheType = type.getSettings().get("cache", indexSettings.get(FIELDDATA_CACHE_KEY, FIELDDATA_CACHE_VALUE_NODE));
if (FIELDDATA_CACHE_VALUE_NODE.equals(cacheType)) {
cache = indicesFieldDataCache.buildIndexFieldDataCache(indexService, index, fieldNames, type);
} else if ("none".equals(cacheType)){
cache = new IndexFieldDataCache.None();
} else {
throw new IllegalArgumentException("cache type not supported [" + cacheType + "] for field [" + fieldNames.fullName() + "]");
}
fieldDataCaches.put(fieldNames.indexName(), cache);
}
fieldLoadingLock.acquire(key);
try {
IndexFieldData.Builder builder = null;
String format = type.getFormat(indexSettings);
if (format != null && FieldDataType.DOC_VALUES_FORMAT_VALUE.equals(format) && !docValues) {
logger.warn("field [" + fieldNames.fullName() + "] has no doc values, will use default field data format");
format = null;
}
if (format != null) {
builder = buildersByTypeAndFormat.get(Tuple.tuple(type.getType(), format));
if (builder == null) {
logger.warn("failed to find format [" + format + "] for field [" + fieldNames.fullName() + "], will use default");
}
}
if (builder == null && docValues) {
builder = docValuesBuildersByType.get(type.getType());
}
if (builder == null) {
builder = buildersByType.get(type.getType());
}
if (builder == null) {
throw new IllegalArgumentException("failed to find field data builder for field " + fieldNames.fullName() + ", and type " + type.getType());
}

fieldData = builder.build(index, indexSettings, fieldType, cache, circuitBreakerService, indexService.mapperService());
loadedFieldData.put(fieldNames.indexName(), fieldData);
IndexFieldDataCache cache = fieldDataCaches.get(fieldNames.indexName());
if (cache == null) {
// we default to node level cache, which in turn defaults to be unbounded
// this means changing the node level settings is simple, just set the bounds there
String cacheType = type.getSettings().get("cache", indexSettings.get(FIELDDATA_CACHE_KEY, FIELDDATA_CACHE_VALUE_NODE));
if (FIELDDATA_CACHE_VALUE_NODE.equals(cacheType)) {
cache = indicesFieldDataCache.buildIndexFieldDataCache(indexService, index, fieldNames, type);
} else if ("none".equals(cacheType)){
cache = new IndexFieldDataCache.None();
} else {
throw new IllegalArgumentException("cache type not supported [" + cacheType + "] for field [" + fieldNames.fullName() + "]");
}
} finally {
fieldLoadingLock.release(key);
fieldDataCaches.put(fieldNames.indexName(), cache);
}

return (IFD) builder.build(index, indexSettings, fieldType, cache, circuitBreakerService, indexService.mapperService());
} finally {
fieldLoadingLock.release(key);
}
return (IFD) fieldData;
}

}
6 changes: 1 addition & 5 deletions core/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
import org.elasticsearch.index.AbstractIndexComponent;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.analysis.AnalysisService;
import org.elasticsearch.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.mapper.Mapper.BuilderContext;
import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
import org.elasticsearch.index.mapper.object.ObjectMapper;
Expand Down Expand Up @@ -104,7 +103,6 @@ public Analyzer apply(MappedFieldType fieldType) {
};

private final AnalysisService analysisService;
private final IndexFieldDataService fieldDataService;

/**
* Will create types automatically if they do not exists in the mapping definition yet
Expand Down Expand Up @@ -139,12 +137,11 @@ public Analyzer apply(MappedFieldType fieldType) {
private volatile ImmutableSet<String> parentTypes = ImmutableSet.of();

@Inject
public MapperService(Index index, @IndexSettings Settings indexSettings, AnalysisService analysisService, IndexFieldDataService fieldDataService,
public MapperService(Index index, @IndexSettings Settings indexSettings, AnalysisService analysisService,
SimilarityLookupService similarityLookupService,
ScriptService scriptService) {
super(index, indexSettings);
this.analysisService = analysisService;
this.fieldDataService = fieldDataService;
this.fieldTypes = new FieldTypeLookup();
this.documentParser = new DocumentMapperParser(indexSettings, this, analysisService, similarityLookupService, scriptService);
this.indexAnalyzer = new MapperAnalyzerWrapper(analysisService.defaultIndexAnalyzer(), INDEX_ANALYZER_EXTRACTOR);
Expand Down Expand Up @@ -291,7 +288,6 @@ private DocumentMapper merge(DocumentMapper mapper, boolean updateAllTypes) {
logger.debug("merging mapping for type [{}] resulted in conflicts: [{}]", mapper.type(), Arrays.toString(result.buildConflicts()));
}
}
fieldDataService.onMappingUpdate();
return oldMapper;
} else {
List<ObjectMapper> newObjectMappers = new ArrayList<>();
Expand Down
3 changes: 2 additions & 1 deletion core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
import static org.elasticsearch.index.engine.Engine.Operation.Origin.PRIMARY;
import static org.elasticsearch.index.engine.Engine.Operation.Origin.REPLICA;
import static org.hamcrest.Matchers.*;

public class InternalEngineTests extends ElasticsearchTestCase {

private static final Pattern PARSE_LEGACY_ID_PATTERN = Pattern.compile("^" + Translog.TRANSLOG_FILE_PREFIX + "(\\d+)((\\.recovering))?$");
Expand Down Expand Up @@ -1923,7 +1924,7 @@ public TranslogHandler(String indexName) {
Index index = new Index(indexName);
AnalysisService analysisService = new AnalysisService(index, settings);
SimilarityLookupService similarityLookupService = new SimilarityLookupService(index, settings);
MapperService mapperService = new MapperService(index, settings, analysisService, null, similarityLookupService, null);
MapperService mapperService = new MapperService(index, settings, analysisService, similarityLookupService, null);
DocumentMapper.Builder b = new DocumentMapper.Builder(settings, rootBuilder, mapperService);
DocumentMapperParser parser = new DocumentMapperParser(settings, mapperService, analysisService, similarityLookupService, null);
this.docMapper = b.build(mapperService, parser);
Expand Down
1 change: 0 additions & 1 deletion core/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ public void testChangeFieldDataFormat() throws Exception {
writer.addDocument(doc);
final IndexReader reader2 = DirectoryReader.open(writer, true);
final MappedFieldType mapper2 = MapperBuilders.stringField("s").tokenized(false).docValues(true).fieldDataSettings(Settings.builder().put(FieldDataType.FORMAT_KEY, "doc_values").build()).build(ctx).fieldType();
ifdService.onMappingUpdate();
ifd = ifdService.getForField(mapper2);
assertThat(ifd, instanceOf(SortedSetDVOrdinalsIndexFieldData.class));
reader1.close();
Expand Down

0 comments on commit f8b741d

Please sign in to comment.