From 3f603bdd87e66ee7725a587fc7582159c367e868 Mon Sep 17 00:00:00 2001 From: Anshum Gupta Date: Thu, 9 Oct 2014 20:07:46 +0000 Subject: [PATCH] SOLR-5986: Fix tests and start returning partial results in case of ExitingReaderException git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1630583 13f79535-47bb-0310-9956-ffa450edef68 --- .../solr/handler/MoreLikeThisHandler.java | 7 +++- .../solr/handler/component/SearchHandler.java | 2 - .../java/org/apache/solr/search/Grouping.java | 4 ++ .../apache/solr/search/SolrIndexSearcher.java | 19 ++++++--- .../solr/search/grouping/CommandHandler.java | 4 ++ .../apache/solr/TestDistributedSearch.java | 29 ++++++-------- .../CloudExitableDirectoryReaderTest.java | 39 +++---------------- .../core/ExitableDirectoryReaderTest.java | 23 +++++------ 8 files changed, 53 insertions(+), 74 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java index f360c05fc7e..049cba869f9 100644 --- a/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java @@ -54,6 +54,8 @@ import org.apache.solr.search.SolrReturnFields; import org.apache.solr.search.SortSpec; import org.apache.solr.search.SyntaxError; import org.apache.solr.util.SolrPluginUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.Reader; @@ -77,6 +79,8 @@ public class MoreLikeThisHandler extends RequestHandlerBase { // Pattern is thread safe -- TODO? share this with general 'fl' param private static final Pattern splitList = Pattern.compile(",| "); + + protected static Logger log = LoggerFactory.getLogger(MoreLikeThisHandler.class); @Override public void init(NamedList args) { @@ -267,8 +271,7 @@ public class MoreLikeThisHandler extends RequestHandlerBase } } } catch (ExitableDirectoryReader.ExitingReaderException ex) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, - "MLTHandler Request took too long during query expansion. Terminating request."); + log.warn( "Query: " + req.getParamString() + "; " + ex.getMessage()); } finally { SolrQueryTimeoutImpl.reset(); } diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index 3495ca65abd..349b3fb820e 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -244,8 +244,6 @@ public class SearchHandler extends RequestHandlerBase implements SolrCoreAware , } } catch (ExitableDirectoryReader.ExitingReaderException ex) { log.warn( "Query: " + req.getParamString() + "; " + ex.getMessage()); - throw new SolrException(ErrorCode.BAD_REQUEST, - "Request took too long during query expansion. Terminating request."); } finally { SolrQueryTimeoutImpl.reset(); } diff --git a/solr/core/src/java/org/apache/solr/search/Grouping.java b/solr/core/src/java/org/apache/solr/search/Grouping.java index ee12482c361..acadc5902b1 100644 --- a/solr/core/src/java/org/apache/solr/search/Grouping.java +++ b/solr/core/src/java/org/apache/solr/search/Grouping.java @@ -27,6 +27,7 @@ import java.util.Map; import java.util.Set; import org.apache.commons.lang.ArrayUtils; +import org.apache.lucene.index.ExitableDirectoryReader; import org.apache.lucene.index.StorableField; import org.apache.lucene.queries.function.FunctionQuery; import org.apache.lucene.queries.function.ValueSource; @@ -452,6 +453,9 @@ public class Grouping { } catch (TimeLimitingCollector.TimeExceededException x) { logger.warn( "Query: " + query + "; " + x.getMessage() ); qr.setPartialResults(true); + } catch (ExitableDirectoryReader.ExitingReaderException e) { + logger.warn( "Query: " + query + "; " + e.getMessage() ); + qr.setPartialResults(true); } } diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java index 546cf257149..4b1024cb456 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java @@ -232,7 +232,10 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable,SolrIn catch( TimeLimitingCollector.TimeExceededException x ) { log.warn( "Query: " + query + "; " + x.getMessage() ); qr.setPartialResults(true); - } + } catch ( ExitableDirectoryReader.ExitingReaderException e) { + log.warn("Query: " + query + "; " + e.getMessage()); + qr.setPartialResults(true); + } } public SolrIndexSearcher(SolrCore core, String path, IndexSchema schema, SolrIndexConfig config, String name, @@ -1217,11 +1220,15 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable,SolrIn protected DocSet getDocSetNC(Query query, DocSet filter) throws IOException { DocSetCollector collector = new DocSetCollector(maxDoc()>>6, maxDoc()); - if (filter==null) { - super.search(query,null,collector); - } else { - Filter luceneFilter = filter.getTopFilter(); - super.search(query, luceneFilter, collector); + try { + if (filter == null) { + super.search(query, null, collector); + } else { + Filter luceneFilter = filter.getTopFilter(); + super.search(query, luceneFilter, collector); + } + } catch ( ExitableDirectoryReader.ExitingReaderException e) { + log.warn("Query: " + query + "; " + e.getMessage()); } return collector.getDocSet(); } diff --git a/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java b/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java index 8e94d4336fb..e82170b1d38 100644 --- a/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java +++ b/solr/core/src/java/org/apache/solr/search/grouping/CommandHandler.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import org.apache.lucene.index.ExitableDirectoryReader; import org.apache.lucene.queries.function.ValueSource; import org.apache.lucene.search.Collector; import org.apache.lucene.search.Filter; @@ -230,6 +231,9 @@ public class CommandHandler { } catch (TimeLimitingCollector.TimeExceededException x) { partialResults = true; logger.warn( "Query: " + query + "; " + x.getMessage() ); + } catch (ExitableDirectoryReader.ExitingReaderException e) { + partialResults = true; + logger.warn( "Query: " + query + "; " + e.getMessage() ); } if (includeHitCount) { diff --git a/solr/core/src/test/org/apache/solr/TestDistributedSearch.java b/solr/core/src/test/org/apache/solr/TestDistributedSearch.java index deff80df58f..43dac0cc05d 100644 --- a/solr/core/src/test/org/apache/solr/TestDistributedSearch.java +++ b/solr/core/src/test/org/apache/solr/TestDistributedSearch.java @@ -503,24 +503,17 @@ public class TestDistributedSearch extends BaseDistributedSearchTestCase { ShardParams.SHARDS_TOLERANT, "true"); // test group query - // TODO: Remove this? This doesn't make any real sense now that timeAllowed might trigger early - // termination of the request during Terms enumeration/Query expansion. - // During such an exit, partial results isn't supported as it wouldn't make any sense. - // Increasing the timeAllowed from 1 to 100 for now. - // - // TODO: still failing in jenkins - see SOLR-5986 - // - // queryPartialResults(upShards, upClients, - // "q", "*:*", - // "rows", 100, - // "fl", "id," + i1, - // "group", "true", - // "group.query", t1 + ":kings OR " + t1 + ":eggs", - // "group.limit", 10, - // "sort", i1 + " asc, id asc", - // CommonParams.TIME_ALLOWED, 100, - // ShardParams.SHARDS_INFO, "true", - // ShardParams.SHARDS_TOLERANT, "true"); + queryPartialResults(upShards, upClients, + "q", "*:*", + "rows", 100, + "fl", "id," + i1, + "group", "true", + "group.query", t1 + ":kings OR " + t1 + ":eggs", + "group.limit", 10, + "sort", i1 + " asc, id asc", + CommonParams.TIME_ALLOWED, 1, + ShardParams.SHARDS_INFO, "true", + ShardParams.SHARDS_TOLERANT, "true"); queryPartialResults(upShards, upClients, "q", "*:*", diff --git a/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java b/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java index 8996e59b46e..b870048cf32 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CloudExitableDirectoryReaderTest.java @@ -20,15 +20,11 @@ package org.apache.solr.cloud; import org.apache.lucene.util.LuceneTestCase.Slow; import org.apache.lucene.util.TestUtil; -import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.response.QueryResponse; -import org.apache.solr.common.SolrException; import org.apache.solr.common.params.ModifiableSolrParams; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.apache.solr.common.SolrException.ErrorCode; - /** * Distributed test for {@link org.apache.lucene.index.ExitableDirectoryReader} */ @@ -36,7 +32,7 @@ import static org.apache.solr.common.SolrException.ErrorCode; public class CloudExitableDirectoryReaderTest extends AbstractFullDistribZkTestBase { public static Logger log = LoggerFactory.getLogger(CloudExitableDirectoryReaderTest.class); private static final int NUM_DOCS_PER_TYPE = 20; - + public CloudExitableDirectoryReaderTest() { configString = "solrconfig-tlog-with-delayingcomponent.xml"; schemaString = "schema.xml"; @@ -74,7 +70,7 @@ public class CloudExitableDirectoryReaderTest extends AbstractFullDistribZkTestB } public void doTimeoutTests() throws Exception { - assertFail(params("q", "name:a*", "timeAllowed", "1")); + assertPartialResults(params("q", "name:a*", "timeAllowed", "1")); /* query rewriting for NUM_DOCS_PER_TYPE terms should take less @@ -86,7 +82,7 @@ public class CloudExitableDirectoryReaderTest extends AbstractFullDistribZkTestB Long timeAllowed = TestUtil.nextLong(random(), fiveSeconds, Long.MAX_VALUE); assertSuccess(params("q", "name:a*", "timeAllowed",timeAllowed.toString())); - assertFail(params("q", "name:a*", "timeAllowed", "1")); + assertPartialResults(params("q", "name:a*", "timeAllowed", "1")); timeAllowed = TestUtil.nextLong(random(), fiveSeconds, Long.MAX_VALUE); assertSuccess(params("q", "name:b*", "timeAllowed",timeAllowed.toString())); @@ -100,37 +96,14 @@ public class CloudExitableDirectoryReaderTest extends AbstractFullDistribZkTestB /** * execute a request, verify that we get an expected error */ - public void assertFail(ModifiableSolrParams p) throws Exception { - String timeoutMessage = "Request took too long during query expansion. Terminating request."; - - try { - ignoreException(timeoutMessage); - queryServer(p); - fail("no exception matching expected: " + ErrorCode.BAD_REQUEST.code + ": " + timeoutMessage); - } catch (SolrServerException e) { - assertTrue("Exception " + e.getCause() + " is not a SolrException:\n" + prettyStackTrace(e.getCause()), - e.getCause() instanceof SolrException); - assertEquals(ErrorCode.BAD_REQUEST.code, ((SolrException)e.getCause()).code()); - assertTrue("Expected error message substr not found: " + timeoutMessage + "