lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Muir" <rcm...@gmail.com>
Subject Re: Review Request 25656: Timeout queries during rewrite/expansion based on timeAllowed parameter
Date Mon, 15 Sep 2014 19:54:41 GMT

-----------------------------------------------------------
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
> 
>


Mime
View raw message