----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25656/#review53390 -----------------------------------------------------------
trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93035> What is the purpose of this? Lucene is an API, it doesnt do logging. it also seems unusued. trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93036> Someone can just use FilterAtomicReader.unwrap() ? trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93037> All of these methods that return in.XXX are unnecessary. FilterAtomicReader does that already. trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93038> This should extend FilterFields. The _ notation is strange. trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93039> This should extend FilterTerms trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93040> This should extend FilterTermsEnum trunk/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93041> I don't agree with the 'global static threadlocal QueryTimeOut' class, why use this approach? This thing can just take an instance instead. trunk/lucene/core/src/java/org/apache/lucene/index/QueryTimeout.java <https://reviews.apache.org/r/25656/#comment93042> There is no way to close this threadlocal (except gc), and i dont think the timeout processing should be done with threadlocal anyway. Instead why cant the reader just take a timeout object? FilterReaders are cheap, you could even make one for each query to keep things simple and contained. trunk/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java <https://reviews.apache.org/r/25656/#comment93043> It looks like this test depends on wall-clock time? Can it be mocked instead? I dont want the false failures because the test "ran too fast" - Robert Muir On Sept. 15, 2014, 7:45 p.m., Anshum Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25656/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2014, 7:45 p.m.) > > > Review request for lucene. > > > Bugs: SOLR-5986 > https://issues.apache.org/jira/browse/SOLR-5986 > > > Repository: lucene > > > Description > ------- > > Latest patch > > > 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/test/org/apache/lucene/index/TestExitableDirectoryReader.java > PRE-CREATION > trunk/solr/core/src/java/org/apache/solr/handler/MoreLikeThisHandler.java > 1625118 > > trunk/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java > 1625118 > trunk/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java > 1625118 > > trunk/solr/core/src/test/org/apache/solr/core/ExitableDirectoryReaderTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/25656/diff/ > > > Testing > ------- > > Manually tested and tests for lucene as well as Solr. > > > Thanks, > > Anshum Gupta > >
