diff --git a/core/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java b/core/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java index f851420e036..287d18c6051 100644 --- a/core/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java +++ b/core/src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java @@ -26,7 +26,6 @@ import org.elasticsearch.common.collect.MapBuilder; 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; @@ -44,7 +43,6 @@ import java.util.ArrayList; 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; @@ -138,7 +136,6 @@ public class IndexFieldDataService extends AbstractIndexComponent { } private final IndicesFieldDataCache indicesFieldDataCache; - private final ConcurrentMap> loadedFieldData = ConcurrentCollections.newConcurrentMap(); private final KeyedLock.GlobalLockable fieldLoadingLock = new KeyedLock.GlobalLockable<>(); private final Map fieldDataCaches = Maps.newHashMap(); // no need for concurrency support, always used under lock @@ -161,15 +158,6 @@ public class IndexFieldDataService extends AbstractIndexComponent { fieldLoadingLock.globalLock().lock(); try { List exceptions = new ArrayList<>(0); - final Collection> fieldDataValues = loadedFieldData.values(); - for (IndexFieldData fieldData : fieldDataValues) { - try { - fieldData.clear(); - } catch (Throwable t) { - exceptions.add(t); - } - } - fieldDataValues.clear(); final Collection fieldDataCacheValues = fieldDataCaches.values(); for (IndexFieldDataCache cache : fieldDataCacheValues) { try { @@ -189,14 +177,6 @@ public class IndexFieldDataService extends AbstractIndexComponent { fieldLoadingLock.acquire(fieldName); try { List 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 { @@ -211,17 +191,6 @@ public class IndexFieldDataService extends AbstractIndexComponent { } } - 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 getForField(MappedFieldType fieldType) { final Names fieldNames = fieldType.names(); @@ -231,57 +200,49 @@ public class IndexFieldDataService extends AbstractIndexComponent { } 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); - } - - fieldData = builder.build(index, indexSettings, fieldType, cache, circuitBreakerService, indexService.mapperService()); - loadedFieldData.put(fieldNames.indexName(), fieldData); - } - } finally { - fieldLoadingLock.release(key); + 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()); + } + + 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); + } + + return (IFD) builder.build(index, indexSettings, fieldType, cache, circuitBreakerService, indexService.mapperService()); + } finally { + fieldLoadingLock.release(key); } - return (IFD) fieldData; } } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java index cf5d9f4edbd..8f20ef5f7ab 100755 --- a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -53,7 +53,6 @@ import org.elasticsearch.common.util.concurrent.ReleasableLock; 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; @@ -104,7 +103,6 @@ public class MapperService extends AbstractIndexComponent { }; private final AnalysisService analysisService; - private final IndexFieldDataService fieldDataService; /** * Will create types automatically if they do not exists in the mapping definition yet @@ -139,12 +137,11 @@ public class MapperService extends AbstractIndexComponent { private volatile ImmutableSet 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); @@ -291,7 +288,6 @@ public class MapperService extends AbstractIndexComponent { logger.debug("merging mapping for type [{}] resulted in conflicts: [{}]", mapper.type(), Arrays.toString(result.buildConflicts())); } } - fieldDataService.onMappingUpdate(); return oldMapper; } else { List newObjectMappers = new ArrayList<>(); diff --git a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java index 6856eb9d0ea..45a730a50f0 100644 --- a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -101,6 +101,7 @@ import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS; 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))?$"); @@ -1923,7 +1924,7 @@ public class InternalEngineTests extends ElasticsearchTestCase { 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); diff --git a/core/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java b/core/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java index 3af20da91ed..2823566fddd 100644 --- a/core/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java +++ b/core/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java @@ -150,7 +150,6 @@ public class IndexFieldDataServiceTests extends ElasticsearchSingleNodeTest { 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();