Merge pull request #12371 from jpountz/fix/mapper_fielddata_dependency

Remove the dependecy on IndexFielddataService from MapperService.

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<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
 
@@ -161,15 +158,6 @@ public class IndexFieldDataService extends AbstractIndexComponent {
         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 {
@@ -189,14 +177,6 @@ public class IndexFieldDataService extends AbstractIndexComponent {
         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 {
@@ -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 extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
         final Names fieldNames = fieldType.names();
@@ -231,12 +200,8 @@ 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) {
@@ -274,14 +239,10 @@ public class IndexFieldDataService extends AbstractIndexComponent {
                 fieldDataCaches.put(fieldNames.indexName(), cache);
             }
 
-                    fieldData = builder.build(index, indexSettings, fieldType, cache, circuitBreakerService, indexService.mapperService());
+            return (IFD) builder.build(index, indexSettings, fieldType, cache, circuitBreakerService, indexService.mapperService());
-                    loadedFieldData.put(fieldNames.indexName(), fieldData);
-                }
         } finally {
             fieldLoadingLock.release(key);
         }
     }
-        return (IFD) fieldData;
-    }
 
 }

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<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);
@@ -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<ObjectMapper> newObjectMappers = new ArrayList<>();

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);

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();