Created: (LUCENE-743) IndexReader.reopen()

IndexReader.reopen()
--------------------

                 Key: LUCENE-743
                 URL: http:
//issues.apache.org/jira/browse/LUCENE-743
             Project: Lucene - Java
          Issue Type: Improvement
          Components: Index
            Reporter: Otis Gospodnetic
            Priority: Minor
         Attachments: IndexReaderUtils.java,
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.
-
If you think it was sent incorrectly contact one of the
administrators: http://issues.apache.org/jira/secure/Administrators.jspa

-
For more information on JIRA, see: http://www.atl
assian.com/software/jira

        

------------------------------------------------------------
---------
To unsubscribe, e-mail: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

     [ http://issues.apache.org/jira/browse/LUCENE-743?page=all
 ]

Otis Gospodnetic updated LUCENE-743:
------------------------------------

    Attachment: IndexReaderUtils.java
                MyMultiReader.java
                MySegmentReader.java

In a direct email to me, Robert said: "All of the files
can be prepended with the ASL."


> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http:
//issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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.
-
If you think it was sent incorrectly contact one of the
administrators: http://issues.apache.org/jira/secure/Administrators.jspa

-
For more information on JIRA, see: http://www.atl
assian.com/software/jira

        

------------------------------------------------------------
---------
To unsubscribe, e-mail: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

    [ http://issues.apache.org/jira/brows
e/LUCENE-743?page=comments#action_12457504 ] 
            
robert engels commented on LUCENE-743:
--------------------------------------

A generic version probably needs to implement reference
counting on the Segments or IndexReader in order to know
when they can be safely closed.

> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http:
//issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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.
-
If you think it was sent incorrectly contact one of the
administrators: http://issues.apache.org/jira/secure/Administrators.jspa

-
For more information on JIRA, see: http://www.atl
assian.com/software/jira

        

------------------------------------------------------------
---------
To unsubscribe, e-mail: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

     [ https://issues.apache.org/jira/browse/LUCENE-743?page=com.
atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Michael Busch reassigned LUCENE-743:
------------------------------------

    Assignee: Michael Busch

> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http
s://issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Assignee: Michael Busch
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

    [ https://issues.apache.org/jira/browse/
LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpan
els:comment-tabpanel#action_12513772 ] 

Hoss Man commented on LUCENE-743:
---------------------------------

i somehow missed seeing this issues before ... i don't
really understand the details, but a few comments that come
to mind...

1) this approach seems to assume that when reopening a
MyMultiReader, the sub readers will all be MySegmentReaders
.. assuming we generalize this to MultiReader/SegmentTeader,
this wouldn't work in the case were people are using a
MultiReader containing other MultiReaders ... not to mention
the possibility of people who have written their own
IndexReader implementations.
in generally we should probably try to approach reopening a
reader as a recursive operation if possible where each type
of reader is responsible for checking to see if it's
underlying data has changed, if not return itself, if so
return a new reader in it's place  (much like rewrite works
for Queries)

2) there is no more commit lock correct? ... is this
approach something that can still be valid using the current
backoff/retry mechanism involved with opening segments?

> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http
s://issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Assignee: Michael Busch
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

    [ https://issues.apache.org/jira/browse/
LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpan
els:comment-tabpanel#action_12514131 ] 

Michael Busch commented on LUCENE-743:
--------------------------------------

> i somehow missed seeing this issues before ... 

actually, me too... first I came across this thread:

http://www.gossamer-thre
ads.com/lists/lucene/java-dev/31592?search_string=refresh;#3
1592

in which Doug suggested adding a static method
IndexReader.open(IndexReader) 
which would either return the passed in IndexReader instance
in case is is
up to date or return a new, refreshed instance. 

I started implementing this, using Dougs and Roberts ideas
and then realized 
that there was already this open issue. But the code here is
quite outdated.

> in generally we should probably try to approach
reopening a reader as a 
> recursive operation

Yeah we could do that. However, this might not be so easy to
implement.
For example, if a user creates a MultiReader instance and
adds whatever
subreaders, we would have to recursively refresh the
underlying readers.
But if the MultiReader was created automatically by
IndexReader.open() just
calling refresh on the subreaders is not enough. New
SegmentReaders have to
be opened for new segments.

Also the recursive walk would have to take place within the
FindSegmentsFile
logic.

I decided therefore to only allow IndexReaders to be
refreshed if they were
created by one of the IndexReader.open() methods. I'm going
to submit a first
version of my patch soon. Do you think this is too limiting?


> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http
s://issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Assignee: Michael Busch
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

     [ https://issues.apache.org/jira/browse/LUCENE-743?page=com.
atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Michael Busch updated LUCENE-743:
---------------------------------

    Attachment: lucene-743.patch

First version of my patch:

   * Adds the static method IndexReader.open(IndexReader)
     that returns a new instance of IndexReader in case
     the reader could be updated. If it was up to date
     then the passed-in instance is returned. Only
     IndexReader instances that were created by one of
     the IndexReader.open() methods can be refreshed.

   * SegmentsReader.reopen(SegmentInfos) looks in the 
     SegmentInfos for a segment with the same name. If
     one could be found then either the deleted docs or
     the norms were updated, otherwise the segment name
     would have changed. reopen() clones the 
     SegmentReader and either loads the deleted docs,
     the norms or both. Then the clone is returned and
     the original SegmentReader is marked as closed.

   * If the index has only one segment, then 
     IndexReader.open(IndexReader) checks if the passed
     in IndexReader can be refreshed. This is only 
     possible if it is no MultiReader and if the segment
     name has not changed. Otherwise a new SegmentReader
     instance is returned and the old reader is closed.
	 
   * If the index has multiple segments, then
     IndexReader.open(IndexReader) creates a new 
     MultiReader and tries to reuse the passed-in
     reader (in case it's a SegmentReader) or its 
     subreaders (in case it's a MultiReader). For new
     segments new SegmentReaders are created. Old 
     readers that couldn't be reused are closed.
   
   * Adds the new testcase TestIndexReaderReopen. It
     includes the method 
     assertIndexEquals(IndexReader, IndexReader) that
     checks whether boths IndexReaders have the same
     content. The testcase creates an index and 
     performes different modifications on that index.
     One IndexReader is refreshed after each index
     modification and compared to a fresh IndexReader
     which is opened after each modification.
	 

This first version is for review and not ready to 
commit. I want to add more extensive tests and 
probably clean up the code.

All tests pass.

> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http
s://issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Assignee: Michael Busch
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

    [ https://issues.apache.org/jira/browse/
LUCENE-743?page=com.atlassian.jira.plugin.system.issuetabpan
els:comment-tabpanel#action_12514379 ] 

Hoss Man commented on LUCENE-743:
---------------------------------


> Yeah we could do that. However, this might not be so
easy to implement.
> For example, if a user creates a MultiReader instance
and adds whatever
> subreaders, we would have to recursively refresh the
underlying readers.
> But if the MultiReader was created automatically by
IndexReader.open() just
> calling refresh on the subreaders is not enough. New
SegmentReaders have to
> be opened for new segments. 

...this being the curse that is MultiReader -- it can serve
two very differenet purposes.  

You seem to have already solved the multisegments in a
single directory approach, the MultiReader over many
subreader part actually seems much easier to me (just call
your open method on all of the subreaders) the only tricky
part is detecting which behavior should be used when. This
could be driven by a simple boolean property of MultiReader
indicating whether it owns it's directory and we need to
look for new segments or not -- in which case we just need
to refresh the subreaders.  (My personal preference would be
to change MultiReader so "this.directory" is null
if it was open over several other subReaders, right now it's
just assigned to the first one arbitrarily, but there may be
other consequences of changing that)

Incidentally: I don't think it's crucial that this be done
as a recursive method.  the same approach i describe could
be added to  static utility like what you've got, I just
think that if it's possible to do it recursively we should
so that  *if* someone does write their own MultiReader or
SegmentReader subclass they can still benefit from any core
reopening logic as long as theey do their part to
"reopen" their extensions.


> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http
s://issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Assignee: Michael Busch
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

     [ https://issues.apache.org/jira/browse/LUCENE-743?page=com.
atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Hoss Man updated LUCENE-743:
----------------------------

    Attachment: lucene-743.patch

an extremely hackish refactoring of the previous patch that
demonstrates the method working recursively and dealing with
MultiReaders constructed over multiple subReaders.

a few notes:

1) no where near enough tests of the subReader situation
2) the refactoring is very very ugly and brutish ... most of
the code is still in IndexReader just because it needs so
many things that are private -- things that really seems
like they should be pushed down into SegmentReader (or made
protected)
3) test triggered an apparent NPE in
MultiReader.isOptimized() when there are subReaders, i
hacked arround this in the test, see usages of
assertIndexEqualsZZZ vs assertIndexEquals
4) the FilterIndexReader situation is ... interesting.  in
theory FilterIndexReader should really be abstract (since if
you aren't subclassing it anyway, why do you want it?) 


> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http
s://issues.apache.org/jira/browse/LUCENE-743
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Index
>            Reporter: Otis Gospodnetic
>            Assignee: Michael Busch
>            Priority: Minor
>         Attachments: IndexReaderUtils.java,
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: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org

     [ https://issues.apache.org/jira/browse/LUCENE-743?page=com.
atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Michael Busch updated LUCENE-743:
---------------------------------

    Fix Version/s: 2.3

> IndexReader.reopen()
> --------------------
>
>                 Key: LUCENE-743
>                 URL: http
s://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.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: java-dev-unsubscribelucene.apache.org
For additional commands, e-mail: java-dev-helplucene.apache.org