From 049430621dfd387151de7612b5966aee35bbc374 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 10 Nov 2015 14:53:32 +0100 Subject: [PATCH] Fix IndexSearcherWrapper interface to not depend on the EngineConfig We only passed the engine config since we had no chance to get the cache etc. from the IndexSearcher. Now that we have these getters we can just pass the searcher instead. --- .../index/shard/IndexSearcherWrapper.java | 16 +++++++--------- .../elasticsearch/index/shard/IndexShard.java | 2 +- .../elasticsearch/index/IndexModuleTests.java | 2 +- .../index/engine/InternalEngineTests.java | 4 ++-- .../index/shard/IndexSearcherWrapperTests.java | 14 +++++++------- .../index/shard/IndexShardTests.java | 7 +++---- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/shard/IndexSearcherWrapper.java b/core/src/main/java/org/elasticsearch/index/shard/IndexSearcherWrapper.java index dff59e9b244..5603001a293 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/IndexSearcherWrapper.java +++ b/core/src/main/java/org/elasticsearch/index/shard/IndexSearcherWrapper.java @@ -27,7 +27,6 @@ import org.apache.lucene.search.IndexSearcher; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; import org.elasticsearch.index.engine.Engine; -import org.elasticsearch.index.engine.EngineConfig; import java.io.IOException; @@ -54,13 +53,11 @@ public class IndexSearcherWrapper { } /** - * @param engineConfig The engine config which can be used to get the query cache and query cache policy from - * when creating a new index searcher * @param searcher The provided index searcher to be wrapped to add custom functionality * @return a new index searcher wrapping the provided index searcher or if no wrapping was performed * the provided index searcher */ - protected IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws IOException { + protected IndexSearcher wrap(IndexSearcher searcher) throws IOException { return searcher; } /** @@ -69,7 +66,7 @@ public class IndexSearcherWrapper { * * This is invoked each time a {@link Engine.Searcher} is requested to do an operation. (for example search) */ - public final Engine.Searcher wrap(EngineConfig engineConfig, Engine.Searcher engineSearcher) throws IOException { + public final Engine.Searcher wrap(Engine.Searcher engineSearcher) throws IOException { final ElasticsearchDirectoryReader elasticsearchDirectoryReader = ElasticsearchDirectoryReader.getElasticsearchDirectoryReader(engineSearcher.getDirectoryReader()); if (elasticsearchDirectoryReader == null) { throw new IllegalStateException("Can't wrap non elasticsearch directory reader"); @@ -87,14 +84,15 @@ public class IndexSearcherWrapper { } } + final IndexSearcher origIndexSearcher = engineSearcher.searcher(); final IndexSearcher innerIndexSearcher = new IndexSearcher(reader); - innerIndexSearcher.setQueryCache(engineConfig.getQueryCache()); - innerIndexSearcher.setQueryCachingPolicy(engineConfig.getQueryCachingPolicy()); - innerIndexSearcher.setSimilarity(engineConfig.getSimilarity()); + innerIndexSearcher.setQueryCache(origIndexSearcher.getQueryCache()); + innerIndexSearcher.setQueryCachingPolicy(origIndexSearcher.getQueryCachingPolicy()); + innerIndexSearcher.setSimilarity(origIndexSearcher.getSimilarity(true)); // TODO: Right now IndexSearcher isn't wrapper friendly, when it becomes wrapper friendly we should revise this extension point // For example if IndexSearcher#rewrite() is overwritten than also IndexSearcher#createNormalizedWeight needs to be overwritten // This needs to be fixed before we can allow the IndexSearcher from Engine to be wrapped multiple times - final IndexSearcher indexSearcher = wrap(engineConfig, innerIndexSearcher); + final IndexSearcher indexSearcher = wrap(innerIndexSearcher); if (reader == nonClosingReaderWrapper && indexSearcher == innerIndexSearcher) { return engineSearcher; } else { diff --git a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java index aad1497c9dc..9c7cf499e50 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -727,7 +727,7 @@ public class IndexShard extends AbstractIndexShardComponent { final Engine.Searcher searcher = engine.acquireSearcher(source); boolean success = false; try { - final Engine.Searcher wrappedSearcher = searcherWrapper == null ? searcher : searcherWrapper.wrap(engineConfig, searcher); + final Engine.Searcher wrappedSearcher = searcherWrapper == null ? searcher : searcherWrapper.wrap(searcher); assert wrappedSearcher != null; success = true; return wrappedSearcher; diff --git a/core/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/core/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 76852621e76..f9ea8579329 100644 --- a/core/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/core/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -385,7 +385,7 @@ public class IndexModuleTests extends ESTestCase { } @Override - public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { + public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { return null; } } 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 eeb3b41dabe..a8acdfa13f8 100644 --- a/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java +++ b/core/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java @@ -519,7 +519,7 @@ public class InternalEngineTests extends ESTestCase { } @Override - public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { + public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { counter.incrementAndGet(); return searcher; } @@ -530,7 +530,7 @@ public class InternalEngineTests extends ESTestCase { engine.close(); engine = new InternalEngine(engine.config(), false); - Engine.Searcher searcher = wrapper.wrap(engine.config(), engine.acquireSearcher("test")); + Engine.Searcher searcher = wrapper.wrap(engine.acquireSearcher("test")); assertThat(counter.get(), equalTo(2)); searcher.close(); IOUtils.close(store, engine); diff --git a/core/src/test/java/org/elasticsearch/index/shard/IndexSearcherWrapperTests.java b/core/src/test/java/org/elasticsearch/index/shard/IndexSearcherWrapperTests.java index 288a691b88b..b66e5e5c491 100644 --- a/core/src/test/java/org/elasticsearch/index/shard/IndexSearcherWrapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/shard/IndexSearcherWrapperTests.java @@ -67,7 +67,7 @@ public class IndexSearcherWrapperTests extends ESTestCase { } @Override - public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { + public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { return searcher; } @@ -76,7 +76,7 @@ public class IndexSearcherWrapperTests extends ESTestCase { final AtomicInteger count = new AtomicInteger(); final AtomicInteger outerCount = new AtomicInteger(); try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { - final Engine.Searcher wrap = wrapper.wrap(ENGINE_CONFIG, engineSearcher); + final Engine.Searcher wrap = wrapper.wrap(engineSearcher); assertEquals(1, wrap.reader().getRefCount()); ElasticsearchDirectoryReader.addReaderCloseListener(wrap.getDirectoryReader(), reader -> { if (reader == open) { @@ -118,13 +118,13 @@ public class IndexSearcherWrapperTests extends ESTestCase { } @Override - public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { + public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { return searcher; } }; final ConcurrentHashMap cache = new ConcurrentHashMap<>(); try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { - try (final Engine.Searcher wrap = wrapper.wrap(ENGINE_CONFIG, engineSearcher)) { + try (final Engine.Searcher wrap = wrapper.wrap(engineSearcher)) { ElasticsearchDirectoryReader.addReaderCloseListener(wrap.getDirectoryReader(), reader -> { cache.remove(reader.getCoreCacheKey()); }); @@ -154,7 +154,7 @@ public class IndexSearcherWrapperTests extends ESTestCase { searcher.setSimilarity(iwc.getSimilarity()); IndexSearcherWrapper wrapper = new IndexSearcherWrapper(); try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { - final Engine.Searcher wrap = wrapper.wrap(ENGINE_CONFIG, engineSearcher); + final Engine.Searcher wrap = wrapper.wrap(engineSearcher); assertSame(wrap, engineSearcher); } IOUtils.close(open, writer, dir); @@ -180,7 +180,7 @@ public class IndexSearcherWrapperTests extends ESTestCase { }; try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { try { - wrapper.wrap(ENGINE_CONFIG, engineSearcher); + wrapper.wrap(engineSearcher); fail("reader must delegate cache key"); } catch (IllegalStateException ex) { // all is well @@ -194,7 +194,7 @@ public class IndexSearcherWrapperTests extends ESTestCase { }; try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { try { - wrapper.wrap(ENGINE_CONFIG, engineSearcher); + wrapper.wrap(engineSearcher); fail("reader must delegate cache key"); } catch (IllegalStateException ex) { // all is well diff --git a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 1c6b80cac79..18dc07d4ef0 100644 --- a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -908,7 +908,7 @@ public class IndexShardTests extends ESSingleNodeTestCase { } @Override - public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { + public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { return searcher; } }; @@ -947,7 +947,7 @@ public class IndexShardTests extends ESSingleNodeTestCase { } @Override - public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { + public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { return searcher; } }; @@ -990,8 +990,7 @@ public class IndexShardTests extends ESSingleNodeTestCase { throw new RuntimeException("boom"); } - @Override - public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { + public IndexSearcher wrap(IndexSearcher searcher) throws EngineException { return searcher; } };