----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25658/#review53638 -----------------------------------------------------------
trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25658/#comment93345> Can we somehow share this with TimeLimitingCollector.TimeExceededException? trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25658/#comment93347> Thanks for removing the redundant methods! However, because this timeout doesnt actually change results, we should probably override the two cache methods getCoreCacheKey()/getCombinedCoreAndDeletesKey() to explicitly call super? Otherwise anything using this won't really support NRT at all. I think these are not handled by FilterAtomicReader by default. The latter should of course really be removed, as it makes no sense, but thats for another issue :) trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25658/#comment93348> Is it possible for the exceptions thrown from this to contain more detailed information? For example, the timeout could contain the timeout value, and maybe also contain "terms=" + this in the exception message, to give a little more context as to where it happened. trunk/lucene/core/src/java/org/apache/lucene/index/QueryTimeout.java <https://reviews.apache.org/r/25658/#comment93351> Do we need this concrete class + QueryTimeOutBase or can we simplify into one thing, maybe just a one-method interface? For java 8 especially this should work pretty intuitively. Just an idea. trunk/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java <https://reviews.apache.org/r/25658/#comment93350> Again I'm concerned about a wall-clock time in tests. If we already have the way to override the guy going into this, can't we just override it to throw exception e.g. on the n'th time its invoked or something so the tests are stable? - Robert Muir On Sept. 16, 2014, 9:40 p.m., Anshum Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25658/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2014, 9:40 p.m.) > > > Review request for lucene. > > > Bugs: SOLR-5986 > https://issues.apache.org/jira/browse/SOLR-5986 > > > Repository: lucene > > > Description > ------- > > Timeout queries when they take too long to rewrite/enumerate over terms. > > > Diffs > ----- > > > trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java > PRE-CREATION > trunk/lucene/core/src/java/org/apache/lucene/index/QueryTimeout.java > PRE-CREATION > trunk/lucene/core/src/java/org/apache/lucene/index/QueryTimeoutBase.java > PRE-CREATION > > trunk/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java > PRE-CREATION > trunk/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java > 1625349 > > trunk/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java > 1625349 > trunk/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java > 1625349 > trunk/solr/core/src/java/org/apache/solr/search/SolrQueryTimeout.java > PRE-CREATION > trunk/solr/core/src/test/org/apache/solr/TestDistributedSearch.java 1625349 > trunk/solr/core/src/test/org/apache/solr/TestGroupingSearch.java 1625349 > > trunk/solr/core/src/test/org/apache/solr/core/ExitableDirectoryReaderTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/25658/diff/ > > > Testing > ------- > > Added Lucene/Solr tests. Tested a bit manually. > > > Thanks, > > Anshum Gupta > >
