diff --git a/server/src/main/java/org/elasticsearch/index/IndexModule.java b/server/src/main/java/org/elasticsearch/index/IndexModule.java index c246fb2b3c9a7..2b69cf89da144 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexModule.java +++ b/server/src/main/java/org/elasticsearch/index/IndexModule.java @@ -23,6 +23,7 @@ import org.apache.lucene.search.similarities.Similarity; import org.apache.lucene.store.MMapDirectory; import org.apache.lucene.util.Constants; +import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.client.Client; @@ -35,6 +36,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.analysis.AnalysisRegistry; +import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.cache.query.DisabledQueryCache; import org.elasticsearch.index.cache.query.IndexQueryCache; import org.elasticsearch.index.cache.query.QueryCache; @@ -388,22 +390,33 @@ public IndexService newIndexService( ? (shard) -> null : indexSearcherWrapper.get(); eventListener.beforeIndexCreated(indexSettings.getIndex(), indexSettings.getSettings()); final IndexStore store = getIndexStore(indexSettings, indexStoreFactories); - final QueryCache queryCache; - if (indexSettings.getValue(INDEX_QUERY_CACHE_ENABLED_SETTING)) { - BiFunction queryCacheProvider = forceQueryCacheProvider.get(); - if (queryCacheProvider == null) { - queryCache = new IndexQueryCache(indexSettings, indicesQueryCache); + QueryCache queryCache = null; + IndexAnalyzers indexAnalyzers = null; + boolean success = false; + try { + if (indexSettings.getValue(INDEX_QUERY_CACHE_ENABLED_SETTING)) { + BiFunction queryCacheProvider = forceQueryCacheProvider.get(); + if (queryCacheProvider == null) { + queryCache = new IndexQueryCache(indexSettings, indicesQueryCache); + } else { + queryCache = queryCacheProvider.apply(indexSettings, indicesQueryCache); + } } else { - queryCache = queryCacheProvider.apply(indexSettings, indicesQueryCache); + queryCache = new DisabledQueryCache(indexSettings); } - } else { - queryCache = new DisabledQueryCache(indexSettings); - } - return new IndexService(indexSettings, environment, xContentRegistry, + indexAnalyzers = analysisRegistry.build(indexSettings); + final IndexService indexService = new IndexService(indexSettings, environment, xContentRegistry, new SimilarityService(indexSettings, scriptService, similarities), - shardStoreDeleter, analysisRegistry, engineFactory, circuitBreakerService, bigArrays, threadPool, scriptService, + shardStoreDeleter, indexAnalyzers, engineFactory, circuitBreakerService, bigArrays, threadPool, scriptService, client, queryCache, store, eventListener, searcherWrapperFactory, mapperRegistry, indicesFieldDataCache, searchOperationListeners, indexOperationListeners, namedWriteableRegistry); + success = true; + return indexService; + } finally { + if (success == false) { + IOUtils.closeWhileHandlingException(queryCache, indexAnalyzers); + } + } } private static IndexStore getIndexStore( diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 319c8fdff5171..f3b4d76d41278 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -44,7 +44,6 @@ import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.env.ShardLock; import org.elasticsearch.env.ShardLockObtainFailedException; -import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.cache.IndexCache; import org.elasticsearch.index.cache.bitset.BitsetFilterCache; @@ -142,8 +141,7 @@ public IndexService( NamedXContentRegistry xContentRegistry, SimilarityService similarityService, ShardStoreDeleter shardStoreDeleter, - AnalysisRegistry registry, - EngineFactory engineFactory, + IndexAnalyzers indexAnalyzers, EngineFactory engineFactory, CircuitBreakerService circuitBreakerService, BigArrays bigArrays, ThreadPool threadPool, @@ -157,14 +155,14 @@ public IndexService( IndicesFieldDataCache indicesFieldDataCache, List searchOperationListeners, List indexingOperationListeners, - NamedWriteableRegistry namedWriteableRegistry) throws IOException { + NamedWriteableRegistry namedWriteableRegistry) { super(indexSettings); this.indexSettings = indexSettings; this.xContentRegistry = xContentRegistry; this.similarityService = similarityService; this.namedWriteableRegistry = namedWriteableRegistry; this.circuitBreakerService = circuitBreakerService; - this.mapperService = new MapperService(indexSettings, registry.build(indexSettings), xContentRegistry, similarityService, + this.mapperService = new MapperService(indexSettings, indexAnalyzers, xContentRegistry, similarityService, mapperRegistry, // we parse all percolator queries as they would be parsed on shard 0 () -> newQueryShardContext(0, null, System::currentTimeMillis, null)); diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 453da095f1ac7..87615be04f1fe 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.index; +import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.index.AssertingDirectoryReader; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.FieldInvertState; @@ -40,12 +41,15 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.PageCacheRecycler; +import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.env.ShardLock; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.index.analysis.AnalysisRegistry; +import org.elasticsearch.index.analysis.AnalyzerProvider; +import org.elasticsearch.index.analysis.AnalyzerScope; import org.elasticsearch.index.cache.query.DisabledQueryCache; import org.elasticsearch.index.cache.query.IndexQueryCache; import org.elasticsearch.index.cache.query.QueryCache; @@ -65,6 +69,7 @@ import org.elasticsearch.index.store.IndexStore; import org.elasticsearch.indices.IndicesModule; import org.elasticsearch.indices.IndicesQueryCache; +import org.elasticsearch.indices.analysis.AnalysisModule; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason; @@ -83,13 +88,17 @@ import java.io.IOException; import java.util.Collections; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.instanceOf; @@ -351,11 +360,19 @@ public void testForceCustomQueryCache() throws IOException { .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("foo", settings); IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, new InternalEngineFactory(), Collections.emptyMap()); - module.forceQueryCacheProvider((a, b) -> new CustomQueryCache()); - expectThrows(AlreadySetException.class, () -> module.forceQueryCacheProvider((a, b) -> new CustomQueryCache())); + final Set liveQueryCaches = new HashSet<>(); + module.forceQueryCacheProvider((a, b) -> { + final CustomQueryCache customQueryCache = new CustomQueryCache(liveQueryCaches); + liveQueryCaches.add(customQueryCache); + return customQueryCache; + }); + expectThrows(AlreadySetException.class, () -> module.forceQueryCacheProvider((a, b) -> { + throw new AssertionError("never called"); + })); IndexService indexService = newIndexService(module); assertTrue(indexService.cache().query() instanceof CustomQueryCache); indexService.close("simon says", false); + assertThat(liveQueryCaches, empty()); } public void testDefaultQueryCacheImplIsSelected() throws IOException { @@ -376,12 +393,73 @@ public void testDisableQueryCacheHasPrecedenceOverForceQueryCache() throws IOExc .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("foo", settings); IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, new InternalEngineFactory(), Collections.emptyMap()); - module.forceQueryCacheProvider((a, b) -> new CustomQueryCache()); + module.forceQueryCacheProvider((a, b) -> new CustomQueryCache(null)); IndexService indexService = newIndexService(module); assertTrue(indexService.cache().query() instanceof DisabledQueryCache); indexService.close("simon says", false); } + public void testCustomQueryCacheCleanedUpIfIndexServiceCreationFails() { + Settings settings = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); + final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("foo", settings); + IndexModule module = new IndexModule(indexSettings, emptyAnalysisRegistry, new InternalEngineFactory(), Collections.emptyMap()); + final Set liveQueryCaches = new HashSet<>(); + module.forceQueryCacheProvider((a, b) -> { + final CustomQueryCache customQueryCache = new CustomQueryCache(liveQueryCaches); + liveQueryCaches.add(customQueryCache); + return customQueryCache; + }); + threadPool.shutdown(); // causes index service creation to fail + expectThrows(EsRejectedExecutionException.class, () -> newIndexService(module)); + assertThat(liveQueryCaches, empty()); + } + + public void testIndexAnalyzersCleanedUpIfIndexServiceCreationFails() { + Settings settings = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()) + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build(); + final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("foo", settings); + + final HashSet openAnalyzers = new HashSet<>(); + final AnalysisModule.AnalysisProvider> analysisProvider = (i,e,n,s) -> new AnalyzerProvider() { + @Override + public String name() { + return "test"; + } + + @Override + public AnalyzerScope scope() { + return AnalyzerScope.INDEX; + } + + @Override + public Analyzer get() { + final Analyzer analyzer = new Analyzer() { + @Override + protected TokenStreamComponents createComponents(String fieldName) { + throw new AssertionError("should not be here"); + } + + @Override + public void close() { + super.close(); + openAnalyzers.remove(this); + } + }; + openAnalyzers.add(analyzer); + return analyzer; + } + }; + final AnalysisRegistry analysisRegistry = new AnalysisRegistry(environment, emptyMap(), emptyMap(), emptyMap(), + singletonMap("test", analysisProvider), emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap()); + IndexModule module = new IndexModule(indexSettings, analysisRegistry, new InternalEngineFactory(), Collections.emptyMap()); + threadPool.shutdown(); // causes index service creation to fail + expectThrows(EsRejectedExecutionException.class, () -> newIndexService(module)); + assertThat(openAnalyzers, empty()); + } + public void testMmapNotAllowed() { String storeType = randomFrom(IndexModule.Type.HYBRIDFS.getSettingsKey(), IndexModule.Type.MMAPFS.getSettingsKey()); final Settings settings = Settings.builder() @@ -400,12 +478,19 @@ public void testMmapNotAllowed() { class CustomQueryCache implements QueryCache { + private final Set liveQueryCaches; + + CustomQueryCache(Set liveQueryCaches) { + this.liveQueryCaches = liveQueryCaches; + } + @Override public void clear(String reason) { } @Override - public void close() throws IOException { + public void close() { + assertTrue(liveQueryCaches == null || liveQueryCaches.remove(this)); } @Override