Mailing-List: contact dev-help@lucene.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@lucene.apache.org
Date: Wed, 10 Apr 2013 03:48:16 +0000 (UTC)
From: "Shai Erera (JIRA)" <jira@apache.org>
To: dev@lucene.apache.org
Message-ID: <JIRA.12542557.1329234248792.140467.1365565696466@arcas>
In-Reply-To: <JIRA.12542557.1329234248792@arcas>
References: <JIRA.12542557.1329234248792@arcas>
Subject: [jira] [Commented] (LUCENE-3786) Create SearcherTaxoManager
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Shai Erera commented on LUCENE-3786:
------------------------------------

Few comments:

* This assert in the test {{assertEquals(1, results.size());}} is kinda moot because we always return a FacetResult, even if empty. Perhaps you can assert that if the acquired reader.maxDoc is > 0, the returned FacetResult.rootNode has at least one child with count that is at least 1?

* Maybe change the end of the test to a single-line IOUtils.close()?

* You wrote previously that the test uses LineFileDocs, but I don't see it. It seems it only adds facets to documents? If so, can it go back to newDirectory()?

* It's good that you identify replaceTaxonomy, makes the code safer.

* TR.getTaxoEpoch: maybe instead of adding it to TR you can use the one on DTW (make it public, @lucene.internal)? It's odd that it documents that this epoch is returned only for an NRT TR, because the epoch is recorded on the taxo index commit data, so conceptually there's no reason why it shouldn't always return it. Yet, since this epoch is used internally, between TW and TR, I prefer not to expose it too much. Hmmm, but then you may hit a false positive where the returned TR is valid, yet just in between the checks the app called replaceTaxo. But I think that's ok since it means the check will fail on the next refresh attempt. Really, if ever DTW.epoch changes, we should fail.

* I don't know how important it is, but perhaps given the short discussion we had above, it would be good to add a 1-liner to decRef why the method seems unprotected, but in reality it's the best we can do?

* In refreshIfNeeded, I understand this code {{newReader.decRef()}} is equivalent to closing {{newReader}} (if epoch has changed). But after I received a question yesterday from a someone who did not understand why we don't call close(), perhaps we should, for clarity?

Otherwise this looks great! When I worked on it in the past, DTR wasn't NRT and the sync was a nightmare. Making it NRT really simplified this manager!
                
> Create SearcherTaxoManager
> --------------------------
>
>                 Key: LUCENE-3786
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3786
>             Project: Lucene - Core
>          Issue Type: New Feature
>          Components: modules/facet
>            Reporter: Shai Erera
>            Assignee: Michael McCandless
>            Priority: Minor
>             Fix For: 5.0, 4.3
>
>         Attachments: LUCENE-3786-3x-nocommit.patch, LUCENE-3786.patch, LUCENE-3786.patch
>
>
> If an application wants to use an IndexSearcher and TaxonomyReader in a SearcherManager-like fashion, it cannot use a separate SearcherManager, and say a TaxonomyReaderManager, because the IndexSearcher and TaxoReader instances need to be in sync. That is, the IS-TR pair must match, or otherwise the category ordinals that are encoded in the search index might not match the ones in the taxonomy index.
> This can happen if someone reopens the IndexSearcher's IndexReader, but does not refresh the TaxonomyReader, and the category ordinals that exist in the reopened IndexReader are not yet visible to the TaxonomyReader instance.
> I'd like to create a SearcherTaxoManager (which is a ReferenceManager) which manages an IndexSearcher and TaxonomyReader pair. Then an application will call:
> {code}
> SearcherTaxoPair pair = manager.acquire();
> try {
>   IndexSearcher searcher = pair.searcher;
>   TaxonomyReader taxoReader = pair.taxoReader;
>   // do something with them
> } finally {
>   manager.release(pair);
>   pair = null;
> }
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

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