lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-3573) TaxonomyReader.refresh() is broken, replace its logic with reopen(), following IR.reopen pattern
Date Wed, 16 Nov 2011 06:34:52 GMT

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

Shai Erera commented on LUCENE-3573:
------------------------------------

Patch looks good ! Few minor comments:

* DirTW: shoudl --> should
* Can we name 'taxoCreateTimeToCommit' just 'taxoCreateTime'?
* OpenMode.CREATE.equals(openMode) can be OpenMode.CREATE == openMode
* Perhaps DirTW.commit() can call commit(null) to reduce code duplication? IndexWriter anyway
calls commit(null) in its commit(), so it's not an error to call iw.commit(null).
** Same for prepareCommit()
* I think that DirectoryTaxonomyReader.DIR_TAXONOMY_CREATE_TIME_PROP should be in DirTW since
it is the one writing it. We also have this notion in other places in the code, i.e. TermInfosWriter
declares the format constants, and other classes reference them.
** Also, how about renaming it to INDEX_CREATE_TIME, since technically it is the index that
has been recreated. This is to avoid confusion when storing this time in e.g. the search index's
commit data, which we do in order to synchronize the taxo and search indices.
* Maybe for debugging add a message to InconsistentTaxonomyException which will tell us that
the taxo has been recreated + the recreate time?

About the other comments:

bq. FilterIndexReader does not implement getCommitUserData(). Once this is fixed can resolvethe
TODO in TestIndexClose. I'll open an issue later.

We need a separate issue to write a test which ensures FilterIndexReader overrides all non-final
methods of IndexReader. But why not fix FIR to override getCommitData in this issue? It's
a simple fix, no need for the TODO (or any change to TestIndexClose) and it is technically
related to the issue, as it exposes a bug in FIR.

bq. TR.refresh() should return a boolean indicating anything was changed (issue).

I prefer that we change the method signature once. Why not modify it to return a boolean?
Isn't that a simple change?

bq. DTW.rollback() seems wrong to me

I added it when TW was changed to extend TwoPhaseCommit. I'll take a look. Thanks for raising
that.
                
> TaxonomyReader.refresh() is broken, replace its logic with reopen(), following IR.reopen
pattern
> ------------------------------------------------------------------------------------------------
>
>                 Key: LUCENE-3573
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3573
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: modules/facet
>            Reporter: Doron Cohen
>            Assignee: Doron Cohen
>            Priority: Minor
>         Attachments: LUCENE-3573.patch, LUCENE-3573.patch
>
>
> When recreating the taxonomy index, TR's assumption that categories are only added does
not hold anymore.
> As result, calling TR.refresh() will be incorrect at best, but usually throw an AIOOBE.

--
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-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message