[ 
https://issues.apache.org/jira/browse/LUCENE-3486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13120405#comment-13120405
 ] 

Shai Erera commented on LUCENE-3486:
------------------------------------

What a cool object, Mike ! And the javadocs are very good too.

* Perhaps instead of "recordTimeSec = System.nanoTime()/1000000000.0;" you can 
use TimeUnit.NANOS.toSeconds? Just for clarity, and get rid of this monstrous 
number :).

* Typo: "such that if the use performs": 'use' --> 'user'

* About this code:
{code}
+    if (tracker == null) {
+      tracker = new SearcherTracker(searcher);
+      if (searchers.putIfAbsent(version, tracker) != null) {
+        // Another thread beat us -- must decRef to undo
+        // incRef done by SearcherTracker ctor:
+        searcher.getIndexReader().decRef();
+      }
{code}
Would it be better if SearcherTracker has a close() method and we call it 
instead of decRef()-ing on our own? Seems cleaner to me, and I always like to 
see the code that incRef/new closer to decRef/close. And if tomorrow 
SearcherTracker needs to clear other things too, we're already covered.

* About this "// nocommit -- maybe make it 'public' that you just decRef?" --> 
do you mean whether we should jdoc that that's all we're doing? If so, why 
commit to just that? I don't think it contributes to the user ...

* I have a problem with the Pruner interface. It has a single method prune 
which takes an IndexSearcher and ageSec (BTW, why is it double and not long?). 
And there's PruneByAge impl. But, what other impls could there be for this 
interface, if not by age?
** On the other hand, I can certainly see someone, perhaps w/ NRT, not wanting 
to keep too many searchers around, and instead of committing to an age, he'll 
want to use a hard number (like, the newest 5 searchers) - that interface makes 
it impossible to impl.

If you think however that pruning by age, is the only scenario that makes 
sense, then I suggest removing the interface and having just the impl. 
Otherwise, perhaps a different interface should be created, one that receives a 
list of searchers, with their age, and returns a list of searchers that should 
be released? Just an idea.

Hmm, now that I read prune(Pruner) jdoc, I can see how someone could impl 
"newest 5 searchers" by just counting up to 5 in its doPrune() calls, because 
prune(Pruner) guarantees that the searchers are passed newest to oldest. But 
still I wonder if the interface is not too limited.

Looks very good !
                
> Add SearcherLifetimeManager, so you can retrieve the same searcher you 
> previously used
> --------------------------------------------------------------------------------------
>
>                 Key: LUCENE-3486
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3486
>             Project: Lucene - Java
>          Issue Type: New Feature
>          Components: core/search
>            Reporter: Michael McCandless
>            Assignee: Michael McCandless
>             Fix For: 3.5, 4.0
>
>         Attachments: LUCENE-3486.patch
>
>
> The idea is similar to SOLR-2809 (adding searcher leases to Solr).
> This utility class sits above whatever your source is for "the
> current" searcher (eg NRTManager, SearcherManager, etc.), and records
> (holds a reference to) each searcher in recent history.
> The idea is to ensure that when a user does a follow-on action (clicks
> next page, drills down/up), or when two or more searcher invocations
> within a single user search need to happen against the same searcher
> (eg in distributed search), you can retrieve the same searcher you
> used "last time".
> I think with the new searchAfter API (LUCENE-2215), doing follow-on
> searches on the same searcher is more important, since the "bottom"
> (score/docID) held for that API can easily shift when a new searcher
> is opened.
> When you do a "new" search, you record the searcher you used with the
> manager, and it returns to you a long token (currently just the
> IR.getVersion()), which you can later use to retrieve the same
> searcher.
> Separately you must periodically call prune(), to prune the old
> searchers, ideally from the same thread / at the same time that
> you open a new searcher.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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

Reply via email to