----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25658/#review53648 -----------------------------------------------------------
trunk/lucene/core/src/java/org/apache/lucene/index/QueryTimeout.java <https://reviews.apache.org/r/25658/#comment93367> You can provide the shouldExit implementation in the abstract class if you make the getTimeoutAt() abstract. trunk/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java <https://reviews.apache.org/r/25658/#comment93366> Is the reset necessary? Would it make sense to just start the SolrQueryTimeout clock right when a request is being serviced and let it run until the ThreadLocal is eventually destroyed? trunk/solr/core/src/java/org/apache/solr/search/SolrQueryTimeout.java <https://reviews.apache.org/r/25658/#comment93365> You may want to consider providing a little more detail in the comments that timeOutAt is the time in the future in nanos. Would it make sense to just pass the timeout offset here and have it calculate the future time within the set method? i.e. pass in 1000ms instead of current time + offset. Or another alternative is to provide a date/calendar object in the future (may be a bit overkill but then you don't need to think twice of if you need to pass in the time in millis or nanos). (Also applies to the QueryTimeout too) - Steve Davids On Sept. 17, 2014, 2:32 a.m., Anshum Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25658/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2014, 2:32 a.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 > >
