[
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: [email protected]
For additional commands, e-mail: [email protected]
