> On Sept. 17, 2014, 3:44 a.m., Steve Davids wrote: > > trunk/lucene/core/src/java/org/apache/lucene/index/QueryTimeout.java, line > > 56 > > <https://reviews.apache.org/r/25658/diff/4/?file=691686#file691686line56> > > > > You can provide the shouldExit implementation in the abstract class if > > you make the getTimeoutAt() abstract. > > Anshum Gupta wrote: > shouldExit() is more intuitive for users to implement when they want > their own custom logic for exiting the queries. > > Steve Davids wrote: > Wouldn't it make sense to just go the interface route then? Not sure what > the abstract class is doing for you at this point.
Done :). > On Sept. 17, 2014, 3:44 a.m., Steve Davids wrote: > > trunk/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java, > > line 258 > > <https://reviews.apache.org/r/25658/diff/4/?file=691690#file691690line258> > > > > 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? > > Anshum Gupta wrote: > the reset is necessary for the explicit removal of the ThreadLocal value. > ThreadLocal doesn't clear itself implicitly up after the lifecycle of the > thread and so the reset is more than necessary. > > Steve Davids wrote: > Brain fart - yes threads can be pooled within the container and may > service multiple requests over the thread life cycle. Even if you wouldn't want to use a pool, ThreadLocal would leak the values for threads that are already done processing the request. So.. - Anshum ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25658/#review53648 ----------------------------------------------------------- On Sept. 17, 2014, 5:25 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, 5:25 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 > >
