[
https://issues.apache.org/jira/browse/LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12532990
]
Hoss Man commented on LUCENE-743:
---------------------------------
Okay, read the patch. I'm on board with the need for Clonable now ... it's all
about isolation. if "r.reopen(false) == r" then the client is responsible for
recognizing that it's got the same instance and can make the judgement call
about reusing the instance or cloning depending on it's needs ... internally in
things like MultiReader we have to assume we need a clone for isolation.
Questions and comments...
1. CompoundFileReader, FieldsReader, IndexReader, and BitVector all have clone
methods where they silently catch and ignore CloneNotSupportedExceptions from
super.clone()... if we don't expect the exception to ever happen, we should
just let the exception propogate up the stack (there is no down side to
declaring that clone() throws CloneNotSupportedException). If we think the
exception might happen, but it's not a big deal if it does and we can ignore
it, then we should put a comment in the catch block to that effect. i'm not
clear which are the cases here.
2. i don't remember if the old patches had this issue as well, but having
"return new FilterIndexReader(reopened);" in FilterIndexReader doesn't really
help anyone using FilterIndexReader -- they're going to want an instance of
their own subclass. to meet the contract of FilterIndexReader, we should
implement all "abstract" methods and delegate to the inner reader - so in
theory do don't need a new instance of FIlterIndexReader, we could just return
in.reopen(closeold) ... my gut says it would be better to leave the method out
entirely so that the default impl which throws UnSupOpEx is used --- that way
subclasses who want to use reopen *must* define their own (instead of getting
confused when their filtering logic stops working after they reopen for the
first time)
3. instead of having an isClonable() method, why don't we just remove the
"implements Clonable" declaration from IndexReader and put it on the concrete
subclasses -- then use "instanceof Cloneable" to determine if something is
clonable? ... for that matter, the only place isCloneSupported is really used
is in IndexReader.clone where an exception is thrown if Clone is not supported
... subclasses which know they are Clonable don't need this from the base
class, subclasses which don't implement Clonable but are used in combination
with core classes whose reopen method requires it could be skiped by the caller
(since they don't implement Clonable) ..
4. javadocs frequently refer to "reopened successfully" and "refreshed
successfully" when what it really means is "reopen() returned a newer/fresher
reader" ... this may confuse people who are use to thinking of "successfully" a
"without error"
5. random thought: are their any synchronization issues we're missing here?
I'm wondering about the case where once thread calls reopen while another
thread is updating norms or deleting docs. is there any chance for
inconsistent state?
> IndexReader.reopen()
> --------------------
>
> Key: LUCENE-743
> URL: https://issues.apache.org/jira/browse/LUCENE-743
> Project: Lucene - Java
> Issue Type: Improvement
> Components: Index
> Reporter: Otis Gospodnetic
> Assignee: Michael Busch
> Priority: Minor
> Fix For: 2.3
>
> Attachments: IndexReaderUtils.java, lucene-743-take2.patch,
> lucene-743.patch, lucene-743.patch, lucene-743.patch, MyMultiReader.java,
> MySegmentReader.java
>
>
> This is Robert Engels' implementation of IndexReader.reopen() functionality,
> as a set of 3 new classes (this was easier for him to implement, but should
> probably be folded into the core, if this looks good).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
