lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sean Timm <tim...@aol.com>
Subject Re: [jira] Commented: (LUCENE-997) Add search timeout support to Lucene
Date Thu, 18 Oct 2007 17:57:20 GMT
Roy,

Thanks for the review and comments.  My comments inline below.

Roy Ward wrote:
> (1) You only added timeouts to:
>
>   public TopDocs search(Weight weight, Filter filter, final int nDocs)
>
> It's confusing if timeout functionality is not also added to:
>
>   public TopFieldDocs search(Weight weight, Filter filter, final int nDocs, 
> Sort sort)
>   
Good catch.  That was an oversight.  The necessary changes were made to 
TopFieldDocCollector.java, but you are right the changes to

  public TopFieldDocs search(Weight weight, Filter filter, final int nDocs, 
Sort sort)

were not in the patch.  I have this fixed locally and will submit a 
patch shortly.
> (2) Estimating the the number of results is a good idea, however it breaks
> some of the code in Hits.java when the Vector of results is not as long as
> expected. This either needs more work or just returning the number or
> results actually found. Perhaps a separate method for getting the estimate
> in the case of partial results would be the way to go.
>   
Is there a test case that shows this breakage, or can you point me to 
the code in Hits.java that my patch causes problems with?  Sorry, I'm 
not seeing it.
> (3) The timer, consisting of a whole lot of millisecond pauses (if the
> resolution is 1) is not accurate (certainly under load). There needs to be
> at least an occasional call to an accurate timer. It would also be better to
> replace getCounter() by something like getMilliseconds() so the caller does
> not need to know the resolution of the timer.
I wouldn't expect anyone to actually use a 1 ms resolution.  That is in 
the provided test case simply because it almost guarantees a timeout 
occurs.  The accuracy of the timeout as long as it is reasonably close 
isn't terribly important.  The typical use case as I see it would be to 
preempt the occasional (< 1%)  queries that take an unreasonable amount 
of time to complete.  For example the timer may be configured for 10 
counts of 100 ms (1 second).  If that isn't preempted until 1.1 seconds 
have elapsed, I think my operations team will still be happy.

I do like your suggestion of getMilliseconds().  It is clearer.  I've 
made this change locally and will submit a patch shortly.

-Sean

---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message