Merge pull request #14654 from s1monw/fix_searcher_wrapper_interface

Fix IndexSearcherWrapper interface to not depend on the EngineConfig
This commit is contained in:
Martijn van Groningen 2015-11-10 23:09:32 +07:00
commit 973c094a5d
6 changed files with 21 additions and 24 deletions

View File

@ -27,7 +27,6 @@ import org.apache.lucene.search.IndexSearcher;
import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader; import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
import org.elasticsearch.index.engine.Engine; import org.elasticsearch.index.engine.Engine;
import org.elasticsearch.index.engine.EngineConfig;
import java.io.IOException; 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 * @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 * @return a new index searcher wrapping the provided index searcher or if no wrapping was performed
* the provided index searcher * the provided index searcher
*/ */
protected IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws IOException { protected IndexSearcher wrap(IndexSearcher searcher) throws IOException {
return searcher; 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) * 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()); final ElasticsearchDirectoryReader elasticsearchDirectoryReader = ElasticsearchDirectoryReader.getElasticsearchDirectoryReader(engineSearcher.getDirectoryReader());
if (elasticsearchDirectoryReader == null) { if (elasticsearchDirectoryReader == null) {
throw new IllegalStateException("Can't wrap non elasticsearch directory reader"); 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); final IndexSearcher innerIndexSearcher = new IndexSearcher(reader);
innerIndexSearcher.setQueryCache(engineConfig.getQueryCache()); innerIndexSearcher.setQueryCache(origIndexSearcher.getQueryCache());
innerIndexSearcher.setQueryCachingPolicy(engineConfig.getQueryCachingPolicy()); innerIndexSearcher.setQueryCachingPolicy(origIndexSearcher.getQueryCachingPolicy());
innerIndexSearcher.setSimilarity(engineConfig.getSimilarity()); innerIndexSearcher.setSimilarity(origIndexSearcher.getSimilarity(true));
// TODO: Right now IndexSearcher isn't wrapper friendly, when it becomes wrapper friendly we should revise this extension point // 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 // 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 // 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) { if (reader == nonClosingReaderWrapper && indexSearcher == innerIndexSearcher) {
return engineSearcher; return engineSearcher;
} else { } else {

View File

@ -727,7 +727,7 @@ public class IndexShard extends AbstractIndexShardComponent {
final Engine.Searcher searcher = engine.acquireSearcher(source); final Engine.Searcher searcher = engine.acquireSearcher(source);
boolean success = false; boolean success = false;
try { 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; assert wrappedSearcher != null;
success = true; success = true;
return wrappedSearcher; return wrappedSearcher;

View File

@ -385,7 +385,7 @@ public class IndexModuleTests extends ESTestCase {
} }
@Override @Override
public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { public IndexSearcher wrap(IndexSearcher searcher) throws EngineException {
return null; return null;
} }
} }

View File

@ -519,7 +519,7 @@ public class InternalEngineTests extends ESTestCase {
} }
@Override @Override
public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { public IndexSearcher wrap(IndexSearcher searcher) throws EngineException {
counter.incrementAndGet(); counter.incrementAndGet();
return searcher; return searcher;
} }
@ -530,7 +530,7 @@ public class InternalEngineTests extends ESTestCase {
engine.close(); engine.close();
engine = new InternalEngine(engine.config(), false); 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)); assertThat(counter.get(), equalTo(2));
searcher.close(); searcher.close();
IOUtils.close(store, engine); IOUtils.close(store, engine);

View File

@ -67,7 +67,7 @@ public class IndexSearcherWrapperTests extends ESTestCase {
} }
@Override @Override
public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { public IndexSearcher wrap(IndexSearcher searcher) throws EngineException {
return searcher; return searcher;
} }
@ -76,7 +76,7 @@ public class IndexSearcherWrapperTests extends ESTestCase {
final AtomicInteger count = new AtomicInteger(); final AtomicInteger count = new AtomicInteger();
final AtomicInteger outerCount = new AtomicInteger(); final AtomicInteger outerCount = new AtomicInteger();
try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { 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()); assertEquals(1, wrap.reader().getRefCount());
ElasticsearchDirectoryReader.addReaderCloseListener(wrap.getDirectoryReader(), reader -> { ElasticsearchDirectoryReader.addReaderCloseListener(wrap.getDirectoryReader(), reader -> {
if (reader == open) { if (reader == open) {
@ -118,13 +118,13 @@ public class IndexSearcherWrapperTests extends ESTestCase {
} }
@Override @Override
public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { public IndexSearcher wrap(IndexSearcher searcher) throws EngineException {
return searcher; return searcher;
} }
}; };
final ConcurrentHashMap<Object, TopDocs> cache = new ConcurrentHashMap<>(); final ConcurrentHashMap<Object, TopDocs> cache = new ConcurrentHashMap<>();
try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { 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 -> { ElasticsearchDirectoryReader.addReaderCloseListener(wrap.getDirectoryReader(), reader -> {
cache.remove(reader.getCoreCacheKey()); cache.remove(reader.getCoreCacheKey());
}); });
@ -154,7 +154,7 @@ public class IndexSearcherWrapperTests extends ESTestCase {
searcher.setSimilarity(iwc.getSimilarity()); searcher.setSimilarity(iwc.getSimilarity());
IndexSearcherWrapper wrapper = new IndexSearcherWrapper(); IndexSearcherWrapper wrapper = new IndexSearcherWrapper();
try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { 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); assertSame(wrap, engineSearcher);
} }
IOUtils.close(open, writer, dir); IOUtils.close(open, writer, dir);
@ -180,7 +180,7 @@ public class IndexSearcherWrapperTests extends ESTestCase {
}; };
try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) {
try { try {
wrapper.wrap(ENGINE_CONFIG, engineSearcher); wrapper.wrap(engineSearcher);
fail("reader must delegate cache key"); fail("reader must delegate cache key");
} catch (IllegalStateException ex) { } catch (IllegalStateException ex) {
// all is well // all is well
@ -194,7 +194,7 @@ public class IndexSearcherWrapperTests extends ESTestCase {
}; };
try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) { try (Engine.Searcher engineSearcher = new Engine.Searcher("foo", searcher)) {
try { try {
wrapper.wrap(ENGINE_CONFIG, engineSearcher); wrapper.wrap(engineSearcher);
fail("reader must delegate cache key"); fail("reader must delegate cache key");
} catch (IllegalStateException ex) { } catch (IllegalStateException ex) {
// all is well // all is well

View File

@ -908,7 +908,7 @@ public class IndexShardTests extends ESSingleNodeTestCase {
} }
@Override @Override
public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { public IndexSearcher wrap(IndexSearcher searcher) throws EngineException {
return searcher; return searcher;
} }
}; };
@ -947,7 +947,7 @@ public class IndexShardTests extends ESSingleNodeTestCase {
} }
@Override @Override
public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException { public IndexSearcher wrap(IndexSearcher searcher) throws EngineException {
return searcher; return searcher;
} }
}; };
@ -990,8 +990,7 @@ public class IndexShardTests extends ESSingleNodeTestCase {
throw new RuntimeException("boom"); throw new RuntimeException("boom");
} }
@Override public IndexSearcher wrap(IndexSearcher searcher) throws EngineException {
public IndexSearcher wrap(EngineConfig engineConfig, IndexSearcher searcher) throws EngineException {
return searcher; return searcher;
} }
}; };