> 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.
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. > 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. Brain fart - yes threads can be pooled within the container and may service multiple requests over the thread life cycle. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25658/#review53648 ----------------------------------------------------------- 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 > >
